スーパー丸投げタイム

BEAT · 投稿記事 by **BEAT** » 14年前

前回の日記から1週間がたってしまった(´・ω・｀)

ソースをスマートにしようとして失敗したものを前の状態の戻そうとして時間食いました。
あとマインクラフトやってました・・・猛省・・・

それはそうと
ソースコードってここに載っけたほうがいいんでしょうか？
それともフォルダにまとめてアップロードした方がいいんでしょうか？

わからないので返信待ってます。

とりあえずソースの現状
①引数もちの関数が無いに等しい。
②グローバル変数使いまくり。
③構造体の、メンバが多い、名前がわかりにくい。
④全体的に何をしているのかわかりにくい。
⑤ポインタなんかなかったんや。

という糞仕様となっております。

【追記】
添付されたファイルで多分動きます。
開発環境はVSC++2010Acaで、DXライブラリつかってます。
すごい汚いです。
すごい汚いです。

投稿記事 by **softya（ソフト屋)** » 14年前

zip圧縮してファイル添付で良いと思います。
多くの人にコメントしてもらいたかったら、Ｃ言語何でも掲示板のほうが良いと思いますが。

BEAT · 投稿記事 by **BEAT** » 14年前

わかりました。圧縮して添付します。

それもいいんですが。あまりこのソースコードさらしたくないですからｗｗ

投稿記事 by **softya（ソフト屋)** » 14年前

添付されたのに気づきませんでした。
返答の形で添付してくださいね～。

今後のために直したほうが良いと思うところを書きます。
※　将来のゲーム業界の就活作品の練習としてのきれいなコードを書くことを心がけてくださいね。

・インデントが適当。こういうのが適当だとバグに対して無頓着だと思われます。

・コメントが不足です。こういう所は結構チェックされますので心がけてください。
　就活作品だけやれば良いと思っていると不慣れなことがモロバレになります。

・やはり引数や戻り値が無いので何が更新されているか分かりずらいです。

・関数名は概ね妥当ですが、scrollは統一性としてmap_scrollの方が好ましいです。

・cppの先頭で関数のプロトタイプ宣言をしているがgame.hにもあるので不要です。

・折角player.cppとenemy.cppに分かれているのでidou.cppは不要です。それぞれの移動はplayer.cppとenemy.cppに移すべきです。

・やたら即値が出てくるのでconst intなど定数化(マップの45,60なんか即値の意味なし)を心がける。
　あとenumも活用すべし。switchで出てくる即値の大体はenum定義できるはず。

・外部に公開すべき情報と内部で閉じておくべき情報を無作為に外部公開(グローバル化)していますね。
　これは共同作業すると致命的なバグを生むでしょう。
次のことを心がけてください。
(1)ファイル内でのみ公開する変数や関数(static)をできるだけ増やす。その為に関数の所属するファイルに付いてもよく考える。
　例えば、map_nowとかは外部公開しなくても何とかなります←このために引数のある関数の出番。
(2)一度グローバル変数(extern変数)をすべてやめて引数にしてください。変数には一度すべてstaticを書いてしまいます。
　その上で引数がややこしすぎる場合はグローバル化を改めて検討します。
(3)関数内で閉じている変数は、ローカル変数にしてください。例えばenemy_f。
　さらに値を保持したければstaticなローカル変数にします。例えばenemy_load。
　ここら辺が無頓着だと変数の寿命や変数のスコープについての知識が不足していると認定されます。
　あるいはポインタがうまく使えない人だと思われるでしょう。

・関数やテーブル化して共通化。

CODE:

同じような処理で計算値が違うだけって処理が多いです。
これと
					player.hantei_attack_x1=player.hantei_x1+22;
					player.hantei_attack_y1=player.hantei_y1+12;
これとか
					player.hantei_attack_x1=player.hantei_x1-15;
					player.hantei_attack_y1=player.hantei_y1+14;
数値が違うだけじゃないですか。
関数にして引数に数値を書くか、値をテーブル化しましょう。
参考。
struct {
	int x1;
	int y1;
	int x2;
	int y2;
} buki_attack_hosei[4] = {
	{ -15,14,14,20 },	//MUKI_DOWN
	{ 22,12,14,20 },	//MUKI_UP
	
};
でこれをつかって処理します。
					int index = player.muki_hosei / 4;
					player.hantei_attack_x1=player.hantei_x1+buki_attack_hosei[index].x1;
					player.hantei_attack_y1=player.hantei_y1+buki_attack_hosei[index].y1;
					player.hantei_attack_x2=player.hantei_attack_x1+buki_attack_hosei[index].x2;
					player.hantei_attack_y2=player.hantei_attack_y1+buki_attack_hosei[index].y2;

とりあえず、このぐらいのソース規模なら直しても対した手間ではありませんので全面修正をされるべきかと。
直したものを見せてもらえば、またコメントします。

BEAT · 投稿記事 by **BEAT** » 14年前

ありがとうございます！

とりあえずアドバイスにしたがって直していきたいと思います。

質問したいのですが。

>(1)ファイル内でのみ公開する変数や関数(static)をできるだけ増やす。
ていうのはヘッダファイルでstaticで宣言して本体を使用するファイルに書くてことですか？（やってみればわかるけど一応質問）

>(2)一度グローバル変数(extern変数)をすべてやめて引数にしてください。変数には一度すべてstaticを書いてしまいます。
前から気になっていたんですが、引数をもたせるってことは呼び出し側、たとえばmain（）内で
引数もち関数を呼びだすってことは渡す変数をmain()内で宣言しないといけないってことですよね？

投稿記事 by **softya（ソフト屋)** » 14年前

>ていうのはヘッダファイルでstaticで宣言して本体を使用するファイルに書くてことですか？（やってみればわかるけど一応質問）

それは一番やっていはいけないstaticの使い方です。staticはcppに書くと思ってください。

>前から気になっていたんですが、引数をもたせるってことは呼び出し側、たとえばmain（）内で
>引数もち関数を呼びだすってことは渡す変数をmain()内で宣言しないといけないってことですよね？

static宣言した変数でも構いませんし、別にすべてmainにある必要もありません。
mainが持っていたほうが分かりやすければ、そうすべきですがあまり多くの変数は出てこないと思います。
必要なタイミングで変数は生成すべきで、必要がなくなれば消えても良いのです。

ファイル内の関数で参照可能な変数はある程度作っても構いません。それでも引数にすることが望ましいです。
ファイル外への参照変数(extern)は基本なしにしてください。

BEAT · 投稿記事 by **BEAT** » 14年前

>それは一番やっていはいけないstaticの使い方です。staticはcppに書くと思ってください。
よく考えたらヘッダに書く意味ないですもんね。馬鹿な質問すいません。

2つ目の質問ちょっとわかりにくかったですね。
>その為に関数の所属するファイルに付いてもよく考える。
この部分ちゃんと理解してませんでした。
関数が所属するファイルがきちんとできていれば、static宣言した変数を使って
ｍａｉｎファイルで呼び出す関数に引数をつけなくてもできますね。

それでも、別々のファイルに存在する関数に同じ変数を渡したいときはmainで宣言するしかないですよね？
的なことが言いたかったんです

一度使っている変数の役割についても整理しないといけませんね。

投稿記事 by **softya（ソフト屋)** » 14年前

>それでも、別々のファイルに存在する関数に同じ変数を渡したいときはmainで宣言するしかないですよね？
>的なことが言いたかったんです

そういう役目の変数は種類は少ないとは思いますが、main.cppで宣言する必要のあるものもあるかも知れません。

投稿記事 by **softya（ソフト屋)** » 14年前

ツイットに気になることが書いてあったので補足。
staticでも出来るだけ積極的に構造体は使いましょう。

BEAT · 投稿記事 by **BEAT** » 14年前

とりあえず今現在ファイル内で閉じてる変数にstatic付けて
即値を大体定数にしました。

ですが、ポインタや、引数や戻り値を使って値を渡していく方法がいまいちわかりません。
全部meinに持ってきて関数を呼び出す時に持たせて処理するのならできるのですが、
それ以外の方法が思いつきません。よろしければ例を教えていただきませんか？

※関数でポインタや引数、戻り値を使う方法はわかります。
なるべくファイル内で閉じるように作る方法がわかりません。

投稿記事 by **softya（ソフト屋)** » 14年前

ファイル内で閉じている場合はファイル内スコープ(関数外)のstatic変数にして参照して構いません。まぁ、最小限にはして欲しいです。

例を上げるのは最新コードが提示されていないので昔のに頼りますが、外部変数として参照されている変数で局所的な扱いでしか登場しない物map_nowとかは処理関数を通じてmap.cpp内に閉じ込めることが可能だと思います。

CODE:

//	player.cpp
			int mpx1 = px1 / MAPCHIP_SIZE;
			int mpy1 = px1 / MAPCHIP_SIZE;　← ここのpx1はバグでは？
			int mpx2 = px2 / MAPCHIP_SIZE;
			int mpy2 = py2 / MAPCHIP_SIZE;
			
			//マップとのあたり判定
			if( map_now[ mpy1 ][ mpx1 ] > 3 ||
				map_now[ mpy2 ][ mpx2 ] > 3 ||
				map_now[ mpy1 ][ mpx2 ] > 3 ||
				map_now[ mpy2 ][ mpx1 ] > 3){

だと

CODE:

//	map.cpp
int map_check(int x1,int y1,int x2,int y2,int atai)
{
	return ( map_now[ y1 ][ x1 ] > atai ||
			map_now[ y2 ][ x2 ] > atai ||
			map_now[ y1 ][ x2 ] > atai ||
	   		map_now[ y2 ][ x1 ] > atai		)
}

//	player.cpp
			//マップとのあたり判定
			if( map_check(mapx,mpy1,mpx2,mpy2,3) ) {

こんな風にしていけばグローバル変数は大分減ると思うのです。

そういう風にしても残りそうな変数を逆に教えてください。
私は、
static struct pl player;
はmainに必要そうな気がしてます。
それでも、map_uapdate(&player);
とか引数で渡せるはずですよ。

		7月 2025
日	月	火	水	木	金	土
		1	2	3	4	5
6	7	8	9	10	11	12
13	14	15	16	17	18	19
20	21	22	23	24	25	26
27	28	29	30	31

スーパー丸投げタイム

スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム

Re: スーパー丸投げタイム