C言語ゲーム製作中 ソース公開

質問

プレイヤーが画面を四方八方に移動までです。コードを一通り見ていただきたいです
現在、定番で見やすいプログラムを意識しています。又、今後質問が宜しく願います

以下よりダウンロードです（実行ファイル、ソースコード(268ステップ)等）
http://gamdev.org/up/img/10406.lzh

環境
OS:VISTS、統合開発環境:VC++2005 EE、言語:C
ライブラリ:DXライブラリ、

-----------------------------------------------------------------
前回の質問時の修正としてコメント、マジックナンバー、メイン3分割等を行った。

ファイルは分割せずに１ファイルにまとめています。
-----------------------------------------------------------------
聞きたい優先順位（下に行くほど無視してもよいです）

１：
以下の164行目と166行目のマジックナンバーを#define or typedef enum or それ以外の方法はあるのか…

　　　　　　　　　　赤、緑、青
SetTransColor( 255 , 255 , 255 ) ; // 透過色を変更

*strclr = GetColor( 255 , 255 , 255 ) ; // 白色の値を取得
２：
WinMain関数で多くのローカル変数を宣言しているが他の関数でやったほうがいいか
struct player ply　や　int KeyStat[KET_MAX]　を他のローカルで宣言するとstaticに
しなければいけないとか考えてしまいます。メインで宣言したほうが都合が良さそうと思っていますが…

他に気になった点、こうしたほうがいい等、色々な意見を願います　<(_ _)>

回答No.3

aris-wiz · ベストアンサー

前回のURLも張っておきますね。
http://oshiete.nikkeibp.co.jp/qa3460149.html

No2さんが指摘されているKeyBoardですが、
渡しているのは配列なので、少し誤認があるようです。

配列にアクセスする場合に、KeyBoardでは
*(KeyStat + KEY_UP) = 0;と行っているのに対し、
PlayerSyoriではpkenGraph[ply->direc]としています。
どちらかに統一する方が良いと思います。

直感的に、渡されるものがポインタであるか、配列であるかを
呼び出し元を確認しないでも良いように、KeyBoardには
int KeyBoard( int *KeyStat, int ArraySize )のように
配列のサイズを渡すようにしたり(今回は特に必要ないですが。。。)
関数の引数説明をコメントとして入れるほうが良いかと。

>１：
用法によると思います。
変更する必要が無ければ
#define SET_COL_RED 255
#define SET_COL_GREEN 255
#define SET_COL_BULE 255
程度で良いでしょうし、動的に変更する必要があるならば、
変数を使う方が良いかもしれませんね。
C++では定数定義はconst変数を使用して行われます。
const int SET_COL_REG = 255;
const int SET_COL_GREEN = 255;
const int SET_COL_BULE = 255;
const変数は定義後の値の代入を許さないので定数として扱われます。
これは殆どがGlobalに定義されるため、Cなどでは
グローバル変数をタブーとする為defineを使っている事が多いです。

>２：
KeyStatやgwafはWinMainの中では使用されていないので、
MainProcで定義することで引数を2つ減らすことが出来ますね。
これらは、状態を保持する必要が無く毎回MainProcの中で
更新されている値です。それ以上のスコープを持っても、
持たなくても、結局MainProcの中で書き換えられているものですので
MainProcで定義しましょうstaticは不要です。
そうしないと、MainProc内で無駄に「*」の使用が増えてしまいますし
逆に関数に渡す時には「&」がつきますが、引数の数はスタックサイズなどにも微妙に影響すると思うので、必要な変数は必要なスコープに
置くように心がける方がいいと思います。

前回、最後に書いた一言は、ズバリNo1さんが指摘している
ところで、今回はply->pspd = P_MV_SPD;とせっかく代入してるのに、
ply->px -= P_MV_SPD ;とここでは定数を使っています。

とこのくらいでしょうか

お礼

過去のURLを張ったほうがいいかと思いました
気づかず…*(KeyStat + KEY_UP) = 0;と一緒の形の方向で
int KeyBoard( int *KeyStat, int ArraySize )という形も
あるのですね。必要な時に使ってみます
＞１
#defineが３つでいいのですね。同じような数字なので
いいのかなと迷っていました。<(_ _)>
＞２
確かにKeyStatとgwafは、その関数以後にしか使われていないから
引数を２つ消すことができる。引数が多いことに困惑していました。
視覚的にもすっきりしてよかったです。関数化のメリット!?に
気づいた感じです！

ply->pspd = P_MV_SPD;に関しては修正します

ものすごく分かりやすい説明、有難うございました。感動です！

回答No.5

yphkz4063

ワーク（変数等）は、ひとつの構造体にまとめてください。
ゲームの場合は、ワークのポインタひとつあればいいです。
原則、こう考えるのがゲームの基本です。
「データ構造」の本当の必要性がわかると、見事なプログラムが書けるようになります。
プログラムより「データ構造」が大事。
ゲームのプログラムは『データ構造で記述できる』のです。
引数も極力少なくします。なしでもいいんですよ。
new、delete、malloc、freeの類は、どうしても必要なとき意外は使わない。
これゲームの定石！。

あと、キーを上と下、右と左、あるいは３個か４個同時に押すとキャラが動いてしまいます。
どうでもいいようなことですが、こういうことを押さえていくことが大事です。
これをとてつもなく短いプログラムで動かなくするにはどうするか？・・・などと考えるのが、小さなことなのですが、ゲームプログラミングの醍醐味です。
そうじゃないと楽しくプログラムできないでしょ。

もひとつ、、、わたしなら・・・、
ゲームシステムを記述する中にはOSに依存する関数は使いません。
別に関数つくって別ファイルに固めますね。
決してレベルの高い話ではないです。
癖をつけておくといいです。
IO（入出力）なんてたくさん関数要りませんから意外とすっきり書けますよ。

お礼

(2)-5
プログラムより「データ構造」が大事。…すごく重い言葉です。データ構造に関しては永遠のテーマな所はあります。
自分の考えで理解できる限りは構造体にまとめたいです。
＞ゲームのプログラムは『データ構造で記述できる』のです。
＞引数も極力少なくします。なしでもいいんですよ。
極論!?しかしながらその考え方の理解しやすいです。しかしながら名言だ！
＞new、delete、malloc、freeの類は、どうしても必要なとき意外は使わない。
使いたいが使い方がいまいち分かっていないです　^^；極力使わない方向で
私が教えてもらった人にも言われたことがない…定石をありがとうございます。＞＜

キーに関する指摘も多いです。なので考えて別の修正をしたいと思います。
楽しくできたらいいな…考えるのが大変で色々な人の教えがあるので楽しいことは楽しいが苦しいことも…^^

OSに依存する関数というのはDXライブラリのことでしょうか。
確かウインドウズのOSだと問題ないと認識していますが現時点では
それでもいいと考えています。ライブラリがないとゲーム作成も難しいと思うので
別ファイルにするという意見が多いので早めの対応を考えています。すっきりできれば尚更…

回答No.4

zwi

ソースを見て気になった事をあげて見ます。
(1)DispFunc
初期化系の関数は、InitDispなど統一した名前にしておかないと後で混乱すします。今の名前だけを見るとまるで表示関数です。

(2)パラメータ
MainProcとかパラメータが多すぎます。カテゴリに分けて構造体にまとめましょう。pkenGraphもstruct player plyの構造体に含めて良いでしょう。理由は、処理を追加するたびにパラメータが増えていくと面倒なだけですし、見通しも悪くなります。
表示系やサウンド系のパラメータについては、(3)で説明します。

(3)プログラム全体の構造。
将来的にはプログラムが巨大化するので、表示やプレイヤー制御などを別のソースファイルに分ける必要が出てきます。それを想定したプログラムを書きましょう。
その場合に機能単位にファイルを分割します。表示系、サウンド系、キャラ制御系などです。メインのソースは、統括的な制御するだけでパラメータなどを出来るだけ受け渡さないように最低限の個数にします。
じゃあ、情報はどこで保持するのかというと各ソースの内部で外に出す必要の無い情報はstaticで保持します。外部に返す情報も構造体などにまとめて個数を減らします。
たとえば、CheckSoundMemとかPlaySoundMemは、それぞれ呼び出し関数を作ってサウンド系ソース内に定義します。こうすることで、SHandleなどはサウンド系ソース内でstaticとして保持していれば良くなるのでMainProcから排除できます。

(4)コメント
今のコメントは、機能面のコメントが少ないです。
PlayerSyoriとかだったら、
//キーは左入力？
//左に移動できたら左に移動。出来なければガードする。
//移動アニメーション処理
とかコメントがほしい。まぁ、人によって意見は違うと思いますけど。
とにかく他の人が一目見て何を処理するところか分かれば合格です。

お礼

(2)-4
(1)確かにInitDispの方が統一感があります！
(2)前の回答でもその指摘がありました。理由も書いてくださり有難うございます。
ようするにプレイヤーに関する変数が増えるたびに構造体に組み込まないと面倒という意味と思いました。

(3)関数分けですか…１つのファイルが、わかりやすくていいかと思ったのですが
分けたほうが学びやすいかも知れないし機能単位で分割していきます。
＞統括的な制御するだけでパラメータなどを出来るだけ受け渡さないように最低限の個数にします。
この辺は難しそうですが…前向きに

＞CheckSoundMemとかPlaySoundMemは、それぞれ呼び出し関数を作ってサウンド系ソース内に
＞定義します。こうすることで、SHandleなどはサウンド系ソース内でstaticとして保持していれば
＞良くなるのでMainProcから排除できます
なんとなくのイメージしかできませんが。よくなりそうなので、ぜひ対応したいと思います。
１つのサウンド系ソース内でstatic　SHandleを宣言してCheckSoundMemとかPlaySoundMemを定義するということか…

(4)前回コメントが多すぎて有害だ！と指摘を受けたため
極力減らすようにしました。キー入力部分に少し加えたいと思います

ありがとうございました。これまでの指摘をどれだけ時間が掛かるかはわかりませんが、対応していきます。m(_ _)m

回答No.2

ushioni

前回、どのようなやりとりがされたかは分かりませんが、
見て気づいたところをだらだらと書いてみます。
＃書き換えの例はコンパイルを通してないので、エラーが出るかもしれません。

--------------------------------------------------------
(1)
PlayerSyori()とかで pkenGraph の値が変わっていませんが、
const指定していないようです。

--------------------------------------------------------
(2)
KeyStatの代入/参照がポインタを使っていて怖いです。
例えば、
int KeyStat;
として、KeyBoard()を
void KeyBoard(int *KeyStat)
{
　　*KeyStat =
　　　　(CheckHitKey( KEY_INPUT_UP ) << KEY_UP) |
　　　　(CheckHitKey( KEY_INPUT_DOWN ) << KEY_DOWN) |
　　　　(CheckHitKey( KEY_INPUT_LEFT ) << KEY_LEFT) |
　　　　(CheckHitKey( KEY_INPUT_RIGHT ) << KEY_RIGHT)
　　;
}
とします。そうすると、
if( *(KeyStat + KEY_LEFT) == 1 )
は、
if( (*KeyStat & (1 << KEY_LEFT)) != 0 )
となります。
こうなれば、KeyStatをPlayerSyori()にポインタで渡す必要もなくなります。

if文が分かりにくければ、
#define IS_LEFT(x) ((x) & (1 << KEY_LEFT))
#define IS_RIGHT(x) ((x) & (1 << KEY_RIGHT))
#define IS_DOWN(x) ((x) & (1 << KEY_DOWN))
#define IS_UP(x) ((x) & (1 << KEY_UP))
とかしとくと間違えにくくなるかもしれません。

--------------------------------------------------------
(3)
さらに、
if( (m_count % P_ANM_CNT) < (P_ANM_CNT / 2) )
が何度も出てくるので、例はあんまり行儀良くないですが、
void PlayerSyori( player *ply, const int *pkenGraph, int *strclr, int KeyStat )
{
　　static int cnt = 0;
　　int dir = 0;
　　//
　　cnt++;
　　if ( cnt == P_ANM_CNT / 2 )
　　{
　　　　dir = 1 - dir;
　　　　cnt = 0;
　　}

　　if( IS_LEFT(KeyStat) != 0 )
　　{
　　　　ply->px -= P_MV_SPD ;
　　　　if( ply->px < 0 ) ply->px = 0 ;
　　　　ply->direc = P_DIR_LEFT_STP + dir;
　　}
　　if( IS_RIGHT(KeyStat) != 0 )
　　{
　　　　ply->px += P_MV_SPD ;
　　　　if( ply->px > WIN_MD_SZ_X - P_IMG_SZ_X ) ply->px = WIN_MD_SZ_X - P_IMG_SZ_X ;
　　　　ply->direc = P_DIR_RIGHT_STP + dir;
　　}
　　if( IS_DOWN(KeyStat) != 0 )
　　{
　　　　ply->py += P_MV_SPD ;
　　　　if( ply->py > WIN_MD_SZ_Y - P_IMG_SZ_Y ) ply->py = WIN_MD_SZ_Y - P_IMG_SZ_Y ;
　　　　ply->direc = P_DIR_DOWN_STP + dir;
　　}
　　if( IS_UP(KeyStat) != 0 )
　　{
　　　　ply->py -= P_MV_SPD ;
　　　　if( ply->py < 0 ) ply->py = 0 ;
　　　　ply->direc = P_DIR_UP_STP + dir;
　　}
　　//プレイヤーの描画
　　DrawGraph( ply->px , ply->py , pkenGraph[ply->direc] , TRUE ) ;
}
とかに置き換えられます。

--------------------------------------------------------
(4)
WinMain() でply、pkenGraph などゲーム中でおそらくずっと
使われるであろう変数が auto変数で定義されています。
スタックが消費され続けてもったいない気がするので、
このへんは static変数として外に出すのはいかがでしょうか。
構造体でまとめてしまうのもいいかもしれません。

--------------------------------------------------------
(5)
PlayerSyori()の"Syori"という名前付けは英語にしたほうがカッコいいです。

--------------------------------------------------------

お礼

(1)(2)pkenGraph[ply->direc]でply->direcの内容が変わっていると思うのですが…
シフトのしくみがよく分かっていないです。これから対応します

(3)ぱっと見た感じ難しそうです。今の方が見やすいかもしれないです。
(4)pkenGraphの要素の数は24、plyは16バイトで
それ以上の領域はとらないと思うのですが…
static変数として外（外部変数？）に出したほうが（ｒｙ
構造体でまとめる。具体的にplyの中にpkenGraphを入れるということか・・・検討します

(5)処理はProcessingという単語でした
PlayerProcという方向で

全体的に理解が難しかったです＞＜勉強します
ありがとうございました。

回答No.1

morigann

おー、前回と質問時間帯が全然違うけど続編が出てる。

気づいた点
・「ply->pspd」使ってない・・・（汗）
・上と下を押しながら右を押すと上を向きながら右に動く（汗）

自分の実力ではほとんどアドバイス出来ませんが、頑張って下さい！

お礼

＞・「ply->pspd」使ってない・・・（汗）
気づかなかった…修正しました！
＞・上と下を押しながら右を押すと上を向きながら右に動く（汗）
これに関しては、曖昧な状態なので…もう一度思案します

できる限り頑張ります。ご意見を有難うございました！