戦闘のプログラム作ってるのですが…

[雷]

戦闘のプログラムを作っているのですが、敵２の攻撃時にエラーになってしまいます。
原因の個所が全く分からないので力添えお願いします。
また、他にご指摘がありましたらそれもお願いします！
ZIPファイル添付

[Justy]

＞原因の個所が全く分からないので力添えお願いします
　ざっと見ましたが、敵２だけじゃなく１でも起こりませんか？

　Random_Numbers(set_chara - 1)

　set_charaが１の場合、内部の処理で "% 0"と０除算が行われています。
　これはまずいです。



＞他にご指摘がありましたらそれもお願いします

　細かく見るといろいろあるのですが、あからさまにまずそうなのは２点です。

・　Random_Numbers() / Round_Off()関数が２つのファイルそれぞれで
（内容は同じですが）関数を定義してます。
　どっちか１つで定義するようにして下さい。
（普通はリンク時に文句を言われるはずなのですが・・・）

　ヘッダに実装を書くのは普通はインライン関数かテンプレート関数くらいで、
Random_Numbers()とかのような普通の関数を複数の cppソースでインクルードする
こと場合、実装をヘッダに書くのは止めた方がいいでしょう。


・　set.hなどヘッダファイルのインクルードガードを書きましょう。
　それをしておかないと、後々インクルード順番とか二重定義で苦しめらる可能性があります。

[雷]

↑修正したファイルです

ご回答ありがとうございます。

＞　Random_Numbers(set_chara - 1)
＞　set_charaが１の場合、内部の処理で "% 0"と０除算が行われています。
＞　これはまずいです。

悩んでいたところが解消されました。


＞　細かく見るといろいろあるのですが、あからさまにまずそうなのは２点です。

もし、お時間があれば細かいところも注意していただけないでしょうか？


＞・　Random_Numbers() / Round_Off()関数が２つのファイルそれぞれで
＞（内容は同じですが）関数を定義してます。
＞　どっちか１つで定義するようにして下さい。
＞（普通はリンク時に文句を言われるはずなのですが・・・）
＞
＞　ヘッダに実装を書くのは普通はインライン関数かテンプレート関数くらいで、
＞Random_Numbers()とかのような普通の関数を複数の cppソースでインクルードする
＞こと場合、実装をヘッダに書くのは止めた方がいいでしょう。

Random_Numbers() / Round_Off() の両方をset.cppに移して、Extern.hでexternの
宣言をするように変えてみましたが、こういう解釈でよかったでしょうか？


＞・　set.hなどヘッダファイルのインクルードガードを書きましょう。
＞　それをしておかないと、後々インクルード順番とか二重定義で苦しめらる可能性があります。

二重定義のエラーで、苦しめられていました。
見てみれば納得できることでしたが、思いつけませんでした。
ご指摘、ありがとうございます。

[Justy]

　コンパイルしてみるとわかりますが、あちこちがおかしくなっています。

・　インクルードガード
　"set.h"は合っていますが、"extern.h"や "character.h"は記述がインクルードガードの間違っているので
中身が全て無効になってしまっています。


・　プロトタイプ宣言
　"extern.h"のプロトタイプ宣言ですが、各行の最後に ";"がないのでエラーになります。



＞set.cppに移して、Extern.hでexternの
＞宣言をするように変えてみましたが、こういう解釈でよかったでしょうか？
　それでいいと思いますが、";"がないのでエラーになっていますし、
set.hはもう使わないのであれば消しておいた方がいいと思います。


＞細かいところも注意していただけないでしょうか

・　extern.hの役割

　なんか見ていると全てを extern.hに集めようとしているように見えます。
　
　まだこれくらいの小規模ならそういうやりかたも選択肢として無くはないとは思いますが、
できれば各 .cppに対応した .hを用意してそれぞれの独立性を高くしておいた方が
後々の為にもいいかと思います。

　battle.cppに対応した battle.hを用意して、Battle_Set()のプロトタイプ宣言を書きます。
　同時に battle.hを battle.cppの先頭でインクルードします。
　で、main.cppは battle.hをインクルードすれば main.cppで Battle_Set()が呼び出せます。

　この方法の利点は修正時のミスが減ったり、別のプロジェクトでの使い回しも楽になるところです。



・　void main()
　mainの戻り値は基本的に intです。


・　グローバル変数 Cと E
　そもそもグローバル変数を使う必要があるのかどうかという疑問もありますが、
これは定義と extern宣言が別々になっていますね。

　管理する上で、別々になっているとミスが発生しやすいです。
http://www.pro.or.jp/~fuji/mybooks/cdiag/index.html
　の第５章で触れられているようなマクロを使って、extern宣言と実体の定義を１行で
やってみてはどうでしょうか。


・　character.hの ifdef MAIN_の中
　main.cppからだけ使えるようにしていますが、ここにそれを書くくらいなら
main.cppに static関数として入れてしまった方がいいのではないでしょうか。


・　scanf
　戻り値を必ずチェックして下さい。
　又、必要なら入力ストリームをクリアして下さい。
　それを行わないと、ユーザーが不正文字（数字入力のところでアルファベットを入力するとか）を
力した段階で異常動作が発生します。


・　C+set_chara
　ポインタと整数値の演算が多数ありますが、少々読みにくいと感じます。
　好みの問題かもしれませんが、(C+set_chara)->Lvと書くくらいなら C[set_chara]->Lvとした方が
判りやすくないでしょうか？

　或いは例えば

[color=#d0d0ff" face="monospace]
static void Battle_Avoid(int set_chara, int set_enemy)
{
    STATUS *ts = C+set_chara;
    int set = ts->AV;
    
    ts->AV = ts->AV*2;
    Battle_Attack(E, set_enemy, C, set_chara);
    ts->AV = set;
}[/color]

　のように一時変数にポインタ値を計算しておいてから使うとか。


・　Battle_Commandのコマンド値
　数値によって if文で分岐していますが、ここで数値を直接直値と比較しています。
　複数の箇所で使いそうな直値は defineか enumで名前を付けておいた方がいいかと思います。
　今後増えたり、値を変更したくなったときに数値を "名前"にしておくと
楽です。

[color=#d0d0ff" face="monospace]#define BTL_CMD_ATTACK   1
#define BTL_CMD_DEFENCE 2
#define BTL_CMD_AVOID   3

    if(set == BTL_CMD_ATTACK){
        Battle_Attack(C, set_chara, E, set_enemy);
    }else if(set == BTL_CMD_DEFENCE){
        Battle_Defence(set_chara, set_enemy);
    }else if(set == BTL_CMD_AVOID){
        Battle_Avoid(set_chara, set_enemy);
    }[/color]

　とか。

[雷]

↑変更後のファイル

ご回答ありがとうございます。

・Battle.hで extern void Battel_Set();を宣言すると、VC++で
『'extern void ' : 予期されないストレージ クラスまたは型指定子です。無視されます』
と表示されてしまいます。
ストレージクラスの方は全く理解できていないのですが、型指定子にvoidはあるはずなので問題は無いと思うのですが…
これは、どのような意味のエラーなのでしょうか？

ストレージクラスは
http://spring.cc.kyushu-u.ac.jp/scp/sys ... l/lr36.htm
３．デフォルトより
static or automatic になっていると思われるのですがどうなんでしょうか？

どの様にすれば解決できるのでしょうか？

以下のものは上記のエラーのため確認が取れて居ないものがあります。
申し訳ございません。

>　コンパイルしてみるとわかりますが、あちこちがおかしくなっています。
申し訳ありませんでした。
アップ後にミスに気づいて治したのですが、再度アップの方法をミスったようです。


>・　extern.hの役割
>　なんか見ていると全てを extern.hに集めようとしているように見えます。
集めた方が楽になると思ったのですが、独立性が必要であり、それを失っている
ことに全く気づいていませんでした

>　まだこれくらいの小規模ならそういうやりかたも選択肢として無くはないとは思いますが、
>できれば各 .cppに対応した .hを用意してそれぞれの独立性を高くしておいた方が
>後々の為にもいいかと思います。

はい、これから気おつけたいと思います。

>　この方法の利点は修正時のミスが減ったり、別のプロジェクトでの使い回しも楽になるところです。
これを聞いたら、やらずに居られません！


>・　character.hの ifdef MAIN_の中
>　main.cppからだけ使えるようにしていますが、ここにそれを書くくらいなら
>main.cppに static関数として入れてしまった方がいいのではないでしょうか。

かなり長くなる予定なので、分散させたいと思いこんな感じで設定しています。
あまり良くない方法だったでしょうか？


>・　scanf
>　戻り値を必ずチェックして下さい。

↓これでチェックが成立していると思うのですがどうでしょうか？
for(scanf_test =0; scanf_test == 0;){
scanf_test = scanf("%d",&set);
if(scanf_test != 1)
scanf_test =0;
}

>・　C+set_chara
>　ポインタと整数値の演算が多数ありますが、少々読みにくいと感じます。
>　好みの問題かもしれませんが、(C+set_chara)->Lvと書くくらいなら C[set_chara]->Lvとした方が
>判りやすくないでしょうか？

はい、その通りです。
ですが、(C+set_chara)をC[set_chara]に置き換えるとエラーとなってしまいます。
どの様にすればそのように使うことが出来るようになるのでしょうか？

>・　Battle_Commandのコマンド値

Win32に移動させる予定で、その時変更しようかどうするか悩んでる最中でした（汗
今現在は、上記の
printf("戦う:1　防御:2　回避:3　逃げる:4\n→");
と対応させて設定を行っていたので、あまり必要性を感じませんでしたので…。

[Justy]

＞予期されないストレージ クラスまたは型指定子です
　これは "character.h"のプロトタイプ宣言に問題がある（宣言の最後に ;がない）からです。


＞あまり良くない方法だったでしょうか
　main.cppだけでしか絶対に使わないのならいいのですが、
今後、main.cpp以外から使いたい、とか或いは そういう xxx.cppだけでしか使わない関数が
別のヘッダ・関数とかで増えていったりすると見づらくなったりしませんか？


＞＞・　scanf
＞＞　戻り値を必ずチェックして下さい
＞↓これでチェックが成立していると思うのですがどうでしょうか？
　戻り値のチェックとしてはＯＫです。
　あとは、数字以外が入力されたときの対処でしょうか。


＞＞(C+set_chara)->Lvと書くくらいなら C[set_chara]->Lvとした方
＞、(C+set_chara)をC[set_chara]に置き換えるとエラー.p
　失礼しました。
　C[set_chara].Lvでした。
　これでうまくいくかと。


＞＞と対応させて設定を行っていたので、あまり必要性を感じませんでしたので
　修正の手間とバグの軽減の為の問題なので、必要になったらでもいいと思います。



　とりあえず、ざっくりとですが全体のインクルードや宣言周りを整理してみました。
http://up.img5.net/index.html
up8900.zip (428KB、オリジナルファイル名 my_source.zip）

　変更点は changeフォルダに unifide diff形式のテキストと変更点を表示した pngファイルを添付してありますが、
ヘッダがすっきりし、重複した記述が減っていると思います。

　基本方針としてはcppに対応した機能・サービスを提供する最小限のヘッダになるようにしてあります。
　
　グローバル変数もステータス用のグローバル変数ヘッダとして gv_status.hに移動しました。
　character.hの関数については１つは main.cppでしか多分使われないので、そちらに移動し、
chara_status()に関してはその処理の内容から他のソースからも呼ばれる可能性が高そうだったので、
関数としてそのまま残しました。
　又、入力周りも数字を入力する専用の関数を utility.hに作ってあります。
　あと、Battle_Start()には未初期化変数へ読みとりのアクセスする可能性があるようですが、
そちらは修正していません。

　ご参考までに。

[雷]

とても凄いものをありがとうございます。

unifide diff形式のテキストは何か見れるソフトがあるのでしょうか？
diffで調べた結果は比較ソフトしか出てこなかったので、見方が良く分かりませんでした。

すばやい回答本当にありがとうございました。

[Justy]

＞unifide diff形式のテキストは何か見れるソフトがあるのでしょうか？
　うーん、ただの比較結果のテキストファイルなのでテキストエディタくらいしか知らないです。


＞diffで調べた結果は比較ソフトしか出てこなかったので、見方が良く分かりませんでした
　pngの方で見て頂くということで（^^;


# 微妙にスペルが間違って(unified diff)ました・・・orz

[雷]

>pngの方で見て頂くということで（^^;

わかりました！
調べてたら、ハイパーリンクがどーたらこーたらとあったので、もしかして何かあるのかな？
と思ったけれど、見つけれなかったので質問したしだいです＾＾；

本当にありがとうございました