RPGをつくっているのですが

[peot]

某有名作品を参考にしてRPGの習作をつくっています。

まだ完成してはいませんが、ひとりで制作しているので
他の方の感想を聞いてみたくなりました。

プログラムの書き方に関することで効率の悪い処理や
間違っている場所などのご意見を頂きたいのですが宜しいでしょうか。
ただ分量がありますので面倒なら流し読みする程度で構いません。

制作環境は Visual C++2008EEとDXライブラリです。
全体の大きさは100～600行程度のヘッダとソースが各25ファイル
ほどになります。

処理速度よりは読みやすさを優先した書き方をしているつもり
なのですが素人なので、おかしな部分もあるかもしれません。
あわせてご意見を貰えると助かります。

http://www1.axfc.net/uploader/so/2802586
pass:roleplay

[softya（ソフト屋)]

ざっと見させていただきました。全体的には纏まっていると思いますので重箱の隅を突く感じですが。
私としてはGameFrameクラスのstateの遷移の仕方が気持ち悪いと思います。終了で次のstateを決めるというのが凄く直感的ではありません。
state自体が次の状態を持っているとか、あちこちのクラスに有るstate自体クラス化も考えたほうが良いかと思います。
※　デザインパターンのstateパターンは必要ないと思います。
他はもう少し見てからコメントします。

[とっち]

せっかくなんで私も気なにった点を・・・

まずWinMain.cppのメイン関数内のGameFrameに関してですが、
これポインタである必要がない気がします

コード:

    GameFrame gf;
    gf.Run();

これでいいと思います。

次にsoundに関してなんですが、
現在GameMainのコンストラクタでsound.SetSoundとサウンドデータの
初期化を行っているようですが、Soundクラスのコンストラクタで
セットするようにしてはどうでしょうか？
同様にgraphicsに関してもCreateCharaImage()やDestroyCharaImage()
はGraphicsクラスのコンストラクタ、デストラクタでやるようなほうがいい気がします。

あと、ところどころchar *を使っているようでしたが
filenameなどの変更してないけないようなものの場合、
const char *かstd::string を使うべきです。

僕も一人で気ままに作っている身なんで大層なことは言えませんが、
ぱっと見てこれくらいのことが思いついたんでお伝えしておきます。

[peot]

>softya（ソフト屋) さん
>>GameFrameクラスのstateの遷移の仕方

確かに、Updateの戻り値にstateを持たせて、変化したら遷移させるほうが
より直感的(?)ですね。

>>あちこちのクラスに有るstate自体クラス化

これは色々なクラスで使っている、クラス内関数ポインタ(?)のことでしょうか。
switch文だらけになるのがイヤで使ってみたのですが、言われてみれば
似たような書き方なので、まとめてクラス化できそうですね。

http://marupeke296.com/DXCLS_MethodExecTemplate.html

こちらのサイト様の方法がそのまんま使えそうです。かなり難しそうですが
頑張ってみます。


>とっちさん
>>WinMain.cppのメイン関数内のGameFrameに関して

最初、GameFrame内の各クラスはnewでのインスタンス化をしていなかった
のでGameFrameごとまとめて動的に確保していたのですが、シーンごとに
確保したほうがよいかなぁと思って今のカタチになりました。
言われてみれば、もうGameFrame自体がいらない気もしますけど
なんとな～く保険として残していました。よくないですね…。
ちなみにこの部分は「１４歳からはじめるC++ゲームプログラミング」から
そのまんまパクってます。

>>GameMainでsound.SetSoundとサウンドデータの初期化

上の事柄にもちょっと関係するのですが、シーンごとにサウンドも
必要な分だけ読み込むようにしたかったのでこうしています。
グラフィックの方も同様の理由です。
と言っても、今のところGameMainでしか使ってませんけど……。
わかりづらくてすみません。

>>const char *かstd::string を使うべき

おっしゃるとおりです。スグ書き換えました。


お二人ともご意見ありがとうございます。いろいろと参考になりました。