プログラマ専用SNS ミクプラ

softya（ソフト屋) さんが書きました:ざっと見立て気づいたことだけ。
1.メソッド名と動作が一致していない。CalcBackGroundでDrawBackGroundしているなど。
2.やたらと引数が多い。なぜ情報を整理できなかったのでしょうか？
3.スマートポインタも検討したほうが良いです。
4.元の龍神録を引きずりすぎていると思います。マジックナンバーが残ったままとか。
5.デザインパターンの適用も検討し見てください。
6.GameSequenceCentralとSequenceCentralが似てますね。
細かい所は見れてませんが参考までに。

softya（ソフト屋) さんが書きました: 何処からを初心者脱出というかですが、こういうことに気を付けられたらでしょうか。
「コードの臭い (code smell) - リファクタリングの必要性を示す兆候」
http://www.crystal-creation.com/softwar ... -smell.htm

Dixq (管理人) さんが書きました:龍神録の館のコードは良くないと断りを入れた上で書きます。
館で紹介しているコードと同じことを書いているのに指摘されるのは納得がいかないでしょうが・・。

> 構造体.flag = 2;

これはコメントなしでは意味が分かりません。
そもそも「フラグ」という言葉を使うべきではなく、フラグとしての変数にはtrueかfalseの2値以外は入れるべきではないです。
.flag = 2;
がそれ以上の操作を無効にすることを示すなら
.isAvailable = false; //有効ではない
や
.isAlive = false; //生きていない
等にしてはいかがでしょう。
コメントなしで、コードの意味がスーっと分かるような形が理想です。

Dixq (管理人) さんが書きました: > RegisterShot
> ShotCalc

文法的な細かなことは気にしなくていいと思いますが、自分の中で決まりを作っておいた方が良いと思います。
上記の場合、形が統一出来ていないですよね。
お勧めは
動詞+名詞(目的語)
でしょうか。
上の二つで言えば上の方ですね。

Dixq (管理人) さんが書きました: > 変数名の書き方
> sound_length
> soundHandle

ハイフンでつなぐか、ラクダ型にするか、どちらかに統一してみては。

> .direction = 2;

どちらの方向を代入したのか分からないので、列挙型を使ってはどうでしょう。

コード:

enum eDirection {
    eLeft,
    eUp,
    eRight,
    eDown,
};
...
.direction = eRight;

Dixq (管理人) さんが書きました: > //shot_numのショットを登録した敵と主人公との角度を返す。
> double ...::ShotAtan2(...)

角度を取得する関数名なら Get...(); にすべきでしょう。

> switch(){ case 0:... case 1:... case2:...

マジックナンバーが多いと何をしているのか分からなくなるので、これも列挙型等で分かりやすく書くと良いと思います。

Dixq (管理人) さんが書きました: > クラス名と重複した関数名

クラス名で既に識別可能な物は改めて関数名に付けなくても良いと思います。
例えば「Enemy::EnemyDamage()」はちょっと冗長ですよね。
基本クラスは自分のことをすると思いますから、他のクラスに対して処理をするような関数名にだけ何か識別できる名前を付けると良いかと思います。

Dixq (管理人) さんが書きました: > struct.hの存在

PlayerDataの定義がPlayer.hではなくstruct.hにあるのは少々抵抗があります。
Playerデータを使いたい人がPlayer.hをincludeして使えるようにすべきではないでしょうか。

Dixq (管理人) さんが書きました: > 継承が無い

みたところ継承しているクラスがありませんでした。
Enemy,Boss,Playerは共通点が多いと思いますので、親クラスを作ってはどうでしょう。
私が実装する時は添付ファイルのような継承関係を作っています。
is-a, has-aを意識した継承関係を作れるようになり、ポリモーフィズムを学んだ上で、デザインパターンを学ぶと良いと思います。

この辺を学ぶための、C++のバイブルとも言える本があるので、紹介します。

softya（ソフト屋) さんが書きました:>3.自分的にはいちいちDeleteしてるシーケンス制御で使ってみようと思っていました。しかし、画面がいつまでたっても切り替わらないなど不具合がおきました。
>　そこで質問ですが純粋仮想関数を含む基底クラスをスマートポインタとして扱うことはやってはいけないことだったのでしょうか？

auto_ptrは、純粋仮想関数を含むと問題があったはずです。
shared_ptrとかなら大丈夫だったはず。　まぁ、確認してみてください。

softya（ソフト屋) さんが書きました: >5.例えばどのクラスをどのようなパターンでsoftyaさんなら適用しますか？さしつかえなければ簡単にでいいので答えていただけますでしょうか？

使うパターンですか？私もC++は独学だけなので仕事で使っている人とは視点が違うと思うのと作る人の趣味、作るゲームの種類で変わると思います。
よく出てきそうなものは、FactoryMethod、シングルトン、stateパターン、Mediatorパターンなど、まぁ使えたり使えなかったりしますが、その試行錯誤も意味がありますよ。
ただ、クラスまみれでゲームを一本丸々組んでいないので余り当てにしないでください。

こちらも参考に。
「デザインパターン習得編」
http://marupeke296.com/DP_main.html

softya（ソフト屋) さんが書きました: >6.似ています。機能は同じでnamespaceが違うだけです。やはり統一すべきでしょうか？
はい。

赤鬼 さんが書きました:人様のコードを評価出来るほど熟練した訳では無いですが少しだけ個人的に気になった事を。

テクスチャを動的確保しているのになぜオーディオや、その他多数動的確保しなかったのか気になりました。
また余計なところで必要の無いヘッダのincludeが多々見受けられました。
これではビルドの際時間が余計に掛かるかと思われます。
（ヘッダファイルの解析はそれほど早くありません。また、プリコンパイルも視野に入れて見てはどうでしょうか？）
後は他の方のを参考にしてください。

softya（ソフト屋) さんが書きました:>ひとつ質問があります。C++の公式といいますか、信頼できるといいますかそういった日本語関数リファレンスページはあるのでしょうか？
>それともVisualStudioなのでmsdnのほうをみるべきなのでしょうか？

公式なのはJISとかISOでしょうね。
JIS規格閲覧のみ無料で可。読みやすいかは疑問。
http://www.jisc.go.jp/app/pager?%23jps. ... ISNO=X3014
MSDNも英語しか信頼出来ない。

ただ、C++11はJIS規格がまだのはず。
「C++11 - Wikipedia」
http://ja.wikipedia.org/wiki/C%2B%2B11

公式ではないですが英語だとこんなのもあります。
「cplusplus.com - The C++ Resources Network」
http://www.cplusplus.com/

ライブラリ限定なら日本語のこんなのも。これは個人の人かなぁ。
「cpprefjp - C++ Library Reference」
https://sites.google.com/site/cpprefjp/

Dixq (管理人) さんが書きました: そういう区別でしたか。
メンバ変数との区別は行って良いと思います。しかしパッと見た限り、私は両者の区別が理解できませんでした。
私が知らないだけかもしれませんが・・。
一応よく見かける方法を２つ紹介します。

①　変数名の先頭にmを付ける

int mHandle;
のように、メンバ変数にはmを付けることでローカル変数や引数と区別します。

②　this->を付ける

メンバ変数は this-> を使って参照します。

①は規格上保証されないから嫌だと言う人もいれば、②は名前が長くなるから嫌だと言う人もいるので、まぁ好みだと思います。

softya（ソフト屋) さんが書きました:>3.自分的にはいちいちDeleteしてるシーケンス制御で使ってみようと思っていました。しかし、画面がいつまでたっても切り替わらないなど不具合がおきました。
>　そこで質問ですが純粋仮想関数を含む基底クラスをスマートポインタとして扱うことはやってはいけないことだったのでしょうか？

auto_ptrは、純粋仮想関数を含むと問題があったはずです。
shared_ptrとかなら大丈夫だったはず。　まぁ、確認してみてください。

SmartPointer.reset( new Title() );
...
SmartPointer.reset( new GameOver() );
...

//デストラクタ
Title::~Title( void )
{
	DX::dxCentral.DeleteAllTexture();
	DX::dxCentral.DeleteAllSound();
}

ISLe さんが書きました:auto_ptrは純粋仮想関数を含んでいても問題無いですよ。

ただauto_ptr自身をコピーして使うとき気を付けなければならないだけです。
コピーしてもコピー元が消えるとき参照先のオブジェクトも解体されるのでその後コピー先から参照すると解体済みのオブジェクトにアクセスしてしまいます。
shared_ptrは内部で作成されたコンテナを共有するのでコピーされたshared_ptrがすべて破棄されるまで参照先のオブジェクトは解体されません。

privateなメンバなどで所有権を移す可能性がない場面では余分なコンテナを作ったりしない分シンプルで便利だと思うのですがC++11では非推奨になってしまいました。

ISLe さんが書きました: ライフサイクルのはっきりしているシーン管理より、リソース管理にこそスマートポインタを活用したほうが良いと思います。

dig さんが書きました:ちなみにshared_ptrの弱点は速度以外に何かあるのでしょうか？
シーケンス制御でshared_ptrを使用したとき、たまに動作を停止したりします。SafeDeleteの時は一度もありませんでした。（私のプログラムまたは環境のせいかもしれませんが）
何か制約があるでしょうか？

ISLe さんが書きました: 内部でコンテナを確保するのでメモリの分断がより多く発生します。
その点で速度低下に繋がると思いますが、make_sharedというコンテナと参照先オブジェクトのメモリを一括で確保する手段も用意されています。

shared_ptrは自身が確保したコンテナを参照しますから、参照先のオブジェクトから新たに作成したshared_ptrとは関連を持ちません。
最初に作成したshared_ptr自身をコピーして使わないと解体済みのオブジェクトにアクセスする不具合を引き起こすことになります。

例えば参照先のオブジェクトが内部で自分自身(this)をshared_ptrで参照したとしても、外部のshred_ptrとは連携しないので自滅を制御することはできません。
なのでわたしはゲームのオブジェクト管理には汎用スマートポインタを使わずに、参照カウンタを自分自身で持つクラスを作って使っています。
そうするとゲームのオブジェクト毎の処理をクラス内で完結させることができます。