分割コンパイル時のグローバル変数の使い方について

[Priest]

C言語でゲームを作っています。
ソースコードは複数に分割し、分割コンパイルとなっております。

main.cpp, draw.cpp, Stage.cppの3つのファイルがあるとします。
main.cppにグローバル変数として、構造体の実体が以下のように定義されています。

コード:

// 構造体実体定義
PC Pc[PCNUM];
CAMERA Camera;
BULLET PcBullet[SHOTMAX];
MAP Map[MAPNUM];
GAME Game;
KEYCON KeyCon,PadCon;
SHADER Shader;
FONT Font;
DRAW Draw[DRAWMAX];
GRAPHHANDLE Handle;
TEAM Team[PCNUM];
NUM Num;
KILLLOG KillLog[KILLLOGMAX];

そして、draw.cppではそれを以下のようにextern宣言して使用しています。

コード:

extern PC Pc[PCNUM];
extern CAMERA Camera;
extern GAME Game;
extern MAP Map[MAPNUM];
extern FONT Font;
extern GRAPHHANDLE Handle;
extern TEAM Team[PCNUM];
extern DRAW Draw[DRAWMAX];
extern KILLLOG KillLog[KILLLOGMAX];
extern NUM Num;

分割コンパイルの利点って他のソースコードの変数を隠蔽できる点だと思っていたのですが、
分割コンパイルは実は初めての試みで、これでとりあえず動いているのでなんとなくいいかなってずっと作業を進めていたのですが、
ぶっちゃけこれでは意味がないですよね？
（一応、draw.cppで使わないものはexternしていないので隠蔽にはなっているといえばなっていると思いますが…）
と気になってきました。

そこで、比較的規模の小さいStage.cppをextern 宣言しないように以下のように置き換えてみました。

コード:

#include "DxLib.h"
#include "Stage.h"

// ステージギミック
void StageGimmick(PC *Pc, GAME *Game){
	OutArea(Pc,Game);
}

// 範囲外に出た時の処理
void OutArea(PC *Pc, GAME *Game){
	for(int i=0;i<PCNUM;i++){

		// 範囲外に出たらダメージ
		if((Pc[i].Pos.x<-STAGESIZE/2||STAGESIZE/2<Pc[i].Pos.x||
			Pc[i].Pos.y<-STAGESIZE/2||STAGESIZE/2<Pc[i].Pos.y||
			Pc[i].Pos.z<-STAGESIZE/2||STAGESIZE/2<Pc[i].Pos.z)&&
			!(Pc[i].State==PS_DEADLY||Pc[i].State==PS_DEAD)){
			Pc[i].Life--;
			Pc[i].Damage++;
			Pc[i].Flag.LastDamageFrame=Game->Frame;

			// HP0になったら死亡予約
			if(Pc[i].Life<=0){
				Pc[i].State=PS_DEADLY;
				Pc[i].Flag.LastDeadlyFrame=Game->Frame;
			    Pc[i].Killed=-1;
			 	Pc[i].Killedby=-1;
			}
		}
	}
}

みなさんから見て、この様な使い方はどうですか？
Stage.cppのように、構造体は全てのソースコードから見えるようにしないようにして、
その構造体が必要なら参照渡しする手法の方がいいのか。はたまた別の方法がいいのか。extern 宣言でオールOKなのか。
正直分かりません。
あと、構造体以外にもmain.cppでグローバル変数が定義してあるのですが、これも引数として渡してあげるべきですか？

分割コンパイル時のグローバル変数の使い方について教えてください。

[nil]

結論から申し上げるとグローバル変数は使うべきではありません。
あと、分割の仕方に違和感を感じます。
オブジェクト指向、という言葉もある通り(C言語にはクラスがないのですが、似たような設計にはできます)
大抵ファイルはオブジェクト=物体ごとにファイルわけをします。

すなわち、Pc,Camera,PcBullet...と言った風に分割します。
大抵の場合はDraw関数をそれぞれのオブジェクトに持たせるため、描画部分のみをそれぞれのオブジェクトから隔離する、といったことはしません。

[softya（ソフト屋)]

涼雅さんの言われるとおりでグローバル変数は極力無くすほうがよい設計です。
大半のことは「ゲームプログラミングの館」の「ゲームプログラミング設計」に書かれていますので参考にして下さい。

注意点として龍神録や古いサンプルは、わざと分かりやさ優先で悪い設計になっているので良い設計のためには参考にしないほうが良いです。

[Priest]

涼雅さん

グローバル変数は使用するべきではないのですね。

なるほど、分割手法について、
Draw（描写）, Control（プレイヤーやCPUの入力受付） , Move（キャラやカメラの移動）, Shot（攻撃全般）, Stage（フィールドの効果）, Direction（演出）, Main のように、行為に対して分割してしまいました。
オブジェクト思考という言葉は存じてますが、意識したことはありませんでした。

ありがとうございます！

softyaさん

ありがとうございます、参考にさせていただきます！

[Priest]

現在、参照渡しや値渡しを駆使してグローバル変数撲滅作戦を遂行中ですが、

コード:

// プロトタイプ宣言
void GameInit(GAME *Game, MAP *Map, FONT *Font, GRAPHHANDLE *Handle, SHADER *Shader);
void KeyConfigInit(KEYCON *KeyCon, KEYCON *PadCon);
void PlayInit(GAME *Game, PC *Pc, CAMERA *Camera, TEAM *Team, NUM *Num, GRAPHHANDLE Handle, int PlayerID);
void PcInit(GAME *Game, PC *Pc, GRAPHHANDLE Handle, int PlayerID);
void CameraInit(CAMERA *Camera, PC MainPlayer);

void GetKeyName(char (*KeyName)[12], char (*PadName)[12]);
int GetHitKeyStateAll2(int *Key);
void GetHitPadStateAll(int *Pad, DINPUT_JOYSTATE *PadState);

void WaitKeyOrPad(int Key[],int Pad[], DINPUT_JOYSTATE *PadState);
int KeyOrPad(int Key[], int Pad[], KEYCON KeyCon, KEYCON PadCon, int InputKey);

void FirstTask(PC *Pc, int Key[], int Pad[], KEYCON KeyCon, KEYCON PadCon);
void LastTask(GAME *Game, PC *Pc, TEAM *Team, KILLLOG *KillLog, DRAW *Draw, NUM *Num);

void ControlPhase(PC *Player, CAMERA *Camera, GAME Game, KEYCON KeyCon, KEYCON PadCon);
void MainPlayerControl(PC *Player, CAMERA *Camera, GAME Game, KEYCON KeyCon, KEYCON PadCon, int PlayerIndex);
void CpuControl(PC *Player, GAME Game, int PlayerIndex);

のように引数がごちゃごちゃしてきて少し不安です。
変数名を整理してもこんなになります。

コード:

// プロトタイプ宣言
void GameInit(GAME *, MAP *, FONT *, GRAPHHANDLE *, SHADER *);
void KeyConfigInit(KEYCON *, KEYCON *);
void PlayInit(GAME *Game, PC *, CAMERA *, TEAM *, NUM *, GRAPHHANDLE, int);
void PcInit(GAME *Game, PC *, GRAPHHANDLE, int);
void CameraInit(CAMERA *, PC);

void GetKeyName(char *[12], char *[12]);
int GetHitKeyStateAll2(int *);
void GetHitPadStateAll(int *, DINPUT_JOYSTATE *);

void WaitKeyOrPad(int [],int [], DINPUT_JOYSTATE *);
int KeyOrPad(int [], int [], KEYCON, KEYCON, int);

void FirstTask(PC *, int [], int [], KEYCON, KEYCON);
void LastTask(GAME *, PC *, TEAM *, KILLLOG *, DRAW * NUM *);

void ControlPhase(PC *, CAMERA *, GAME, KEYCON, KEYCON);
void MainPlayerControl(PC *, CAMERA *, GAME, KEYCON, KEYCON, int);
void CpuControl(PC *, GAME, int);

方向性としては間違っていないですよね？

[softya（ソフト屋)]

まず、オブジェクト単位をファイルに纏めないと整理は出来ないでしょうね。
で、オブジェクトに必要な変数は出来るだけオブジェクト内のファイルスコープ変数にしてしまいます。
その過程で、変数で公開するものと非公開なものを明確に分けることになるでしょう。で、公開する物を極力減らすのが常道です。
なんか引数で配列やらポインタが多いので、うまく出来ていないのでは？と思われます。

【追記】
mainが内容を知らなくて良い変数は、mainは情報を保持してはいけません。
これはグローバル変数と変わらないからです。

[Priest]

オブジェクトに必要な変数は出来るだけオブジェクト内のファイルスコープ変数にしてしまいます。

なるほど、とりあえずグローバル変数を消してからオブジェクトを整理するかと思ってましたが、逆なんですね。
やっぱり変ですよね。

理解しました。ありがとうございました！

[Priest]

お二人方の返答を参考にしながら修正してみたのですが、いかがでしょうか？

修正前と修正後のコードを添付してみます。
分割している意味がこれで出てきているのでしょうか？

もし変なところがありましたら教えて頂きたく思います。

修正内容：
extern宣言廃止
コードの分割に関して、オブジェクト指向を意識

【追記】
修正前にあたるものが修正し始めた時の中途半端なものでした。
main内がおかしいのでご了承を・・・。

[softya（ソフト屋)]

変更後のやつで、とりあえず集めた感じのものが結構ありますね。
Num.cpp
Shader.cpp
Team.cpp
名前と中身の対応が分かりづらいです。
あとgame.cppにメニューなどがいるのもごった煮感がします。

それと演出繋がりでDirection.cppになっているんだと思いますが、無理して一つのファイルにまとめないほうが良いのでは？

これだけあってstaticなファイルスコープ関数がないのもなんだか不思議です。
あと、公開している構造体の多さも気になるところですね。
うまく隠蔽すると公開する構造体自体が減るはずです。
PCとか公開する必要がないんじゃないでしょうか？ →　データの抽象化が不十分でファイル間の結合度が高い証拠かと。

参考：「モジュールの強度と結合度＜システムの調達＜Ｗｅｂ教材＜木暮」
http://www.kogures.com/hitoshi/webtext/ ... index.html

[Priest]

ありがとうございます！

変更後のやつで、とりあえず集めた感じのものが結構ありますね。
Num.cpp
Shader.cpp
Team.cpp

適切なところへまとめようと思います。

あと、公開している構造体の多さも気になるところですね。
うまく隠蔽すると公開する構造体自体が減るはずです。
PCとか公開する必要がないんじゃないでしょうか？ →　データの抽象化が不十分でファイル間の結合度が高い証拠かと。

なんとかPCを隠蔽できるように試行錯誤してみます。

他のご指摘も順次対応してみようと思います！

[Priest]

上記URLに書いてあるモジュール結合度でデータ結合になるように変更を加え続けています。
そこで疑問に思ったのですが、

コード:

Function(void){
 int GameFrame=GetGameFrame();

// 処理
}

Function(int GameFrame){

// 処理
}

上記の二つの関数はやっていることはほぼ同じなのですが、引数で渡す方法とGet関数を作って返り値で受け取るのはどちらが推奨されますか？

[softya（ソフト屋)]

GetGameFrame();で統一したほうが良いかと思いますが、最終的には全体を見ての判断ですかね。
どちらが分かりやすいかって事なのですが。そういう意味では両方ありなのかもしれません。

[Priest]

結合度的には同じってことでいいですよね？

ありがとうございます！

[ISLe]

コード:

Function(void){
 int GameFrame=GetGameFrame();

// 処理
}

Function(int GameFrame){

// 処理
}

上記の二つの関数はやっていることはほぼ同じなのですが、引数で渡す方法とGet関数を作って返り値で受け取るのはどちらが推奨されますか？

わたしは後者ですね。
中身にもよりますが、引数だけで高速再生・低速再生とか実装できると思うので。
GetGameFrameに依存するといろんなところで使ってるうちに影響度が掴めなくなると思いますし調べるのも面倒です。

[Priest]

ありがとうございます！

やはり人によって意見が分散する話題になりますね。

現段階では私にはどちらがいいのか判断が下せませんが、そのような利点があることを頭の片隅に置いときつつ
作業を進めようと思います！