当たり判定が上手く作成されない

[Thelly]

初投稿です

現在、横スクロールアクションを製作中で、地形との当たり判定を計算する関数を作成しています。関数の中身はこうなっています。

コード:

/*----------------------------------------------
arx は、キャラクタのx座標,aryはキャラクタのy座標,キャラの当たり判定は64*64の正方形です。
ari,arjはマップ配列map[i][j]を参照するためのものです,マップ配列は64*64のフィールドを1要素としており、そのフィールドの地形情報を持っています。
CMapはマップのクラスです
*/----------------------------------------------
void Control(int* arx,int* ary,int* ari,int*arj,CMap* M)
{
/*map配列の1要素は64*64のフィールドを表しています*/
*arj=int(*arx/64),*ari=int(*ary/64);

/*MXはフィールドのX座標の最大値です。*/

/*ここから当たり判定
当たり判定の判定の仕方は、移動先のmap要素の状態を調べて、陸地なら移動しないようにしています*/

if(Key[KEY_INPUT_RIGHT]>=1 && *arx<MX)
				{
*arx += RUN;
*arj=int(*arx/64),*ari=int(*ary/64);

/*右上と右下の角を調べています Maprffect関数はmap配列の地形情報を返してくれます、1であれば陸地です*/
if((*M).Mapeffect(ari,&(*arj=int((*arx+63)/64)) ) == 1 || (*M).Mapeffect(&(*ari = int((*ary+63)/64)),&(*arj =int((*arx+63)/64))) == 1 ) *arx -= RUN;
				}

if(Key[KEY_INPUT_LEFT]>=1 && *arx>0)
				{
*arx -= RUN;
*arj=int(*arx/64),*ari=int(*ary/64);
/*左上と左下の角をしらべます*/
if((*M).Mapeffect(ari,arj) == 1 || (*M).Mapeffect(&(*ari = int((*ary+63)/64)),arj) == 1 ) *arx += RUN;
				}

if(Key[KEY_INPUT_DOWN]>=1 && *ary<MY)
{
*ary += RUN;
*arj=int(*arx/64),*ari=int(*ary/64);
/*左下と右下の角を調べます*/
if( (*M).Mapeffect(&(*ari = int((*ary+63)/64)),&(*arj =int((*arx+63)/64))) == 1 || (*M).Mapeffect(&(*ari = int((*ary+63)/64)),arj) == 1) *ary -= RUN;
}

if(Key[KEY_INPUT_UP]>=1 && *ary>0)
{
*ary -= RUN;
*arj=int(*arx/64),*ari=int(*ary/64);
/*左上と右上の角を調べます*/
if((*M).Mapeffect(ari,arj) == 1 ||(*M).Mapeffect(ari,&(*arj =int((*arx+63)/64)) ) == 1) *ary += RUN; 
}
	
}

当たり判定の形状は正方形なので、移動時に先行してぶつかる角の判定を調べれば、当たり判定の計算はできます。
現に、右、左、上の判定は正しく行われています。
しかし下の判定が上手く行きません。
左下の角が地面に触れていて、右下の角が地面に触れていないときに、陸地にめり込んでしまいます。

コードを同様に書いたのにこんなことになってしまったので、よくわからなくなり投稿しました。
なにかアドバイスがありましたら返答願います。
お願いします。

[softya（ソフト屋)]

インデント(字下げ)を整形してください。非常に見づらいです。
それと63やら64やらマジックナンバー続出とか、長い条件のifとかポインタ参照が多いとかやる意味のない,演算子とか式に中に代入文があるとか非常に悪い所が多いので、これもプログラムを追いかけづらい原因かと思います。

まず、スッキリさせましょう。
０．インデントをちゃんとする。
１．１つの行には代入と他の処理を同時に書かない。
２．63やら64やらはconst intで変数名にする。
３．63やら64やらの処理は別関数に分ける。
４．if文はかならず{}を書く。
５．RUNの説明をする。
６．arjやariなどという意味不明な名前は避けてください。もっと名前の意味を考えましょう。
arx,aryの座標から配列の添字に変換したものだと意味を込めた名前でお願いします。
７．複雑な条件式は、なるべく簡単になるように書くのと、改行も含めて見やすくなるように整理する。
８．ari,arjのポインタ参照がほんとうに必要でしょうか？arx,aryから求められるんですよね？

[Thelly]

遅くなってすみません
プログラムを修正してみたので見てください

コード:

void Control(int* char_x,int* char_y,CMap* MAP)
{
	
	int hitjudge1 = 0; //当たり判定1
	int hitjudge2 = 0; //当たり判定2
	int map_i_converty =0; //マップ地形取得用変数1 char_yから変換
	int map_j_convertx =0; //マップ地形取得用変数2 char_xから変換
	const int squareside = AREASIZE - 1;//当たり判定の1辺の大きさ,マップの1ブロックの辺の大きさAREASIZE(1辺　64)より1小さいです
	/*右移動の時*/
	if(Key[KEY_INPUT_RIGHT]>=1 && *char_x<MAX_X)
	{
		//右に移動するために、キャラクターのx座標を走っている時の1フレーム移動速度RUN分進めます
		*char_x += RUN;
		/*移動後の当たり判定の判定は、頂点の座標でのマップの地形情報がどのようになっているかで行っています*/
		//右上の頂点の地形情報を調べます
		map_i_converty = int( *char_y/AREASIZE );
		map_j_convertx = int( (*char_x+squareside)/AREASIZE );
		/*Mapeffect関数は、マップの地形情報を(64*64)の範囲を1要素として扱っている配列map[map_i][map_j]の地形情報を
		値として返してくれる関数です、map[map_i][map_j]が陸地のときは1が返ってきます*/
		hitjudge1 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );
		//右下の頂点の地形情報を同様に調べます
		map_i_converty = int( (*char_y+squareside)/AREASIZE );
		map_j_convertx = int( (*char_x+squareside)/AREASIZE );
		hitjudge2 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );
		//もしどちらかが陸地であれば、地面と接触しているので移動をキャンセルします
		if( hitjudge1 == 1 || hitjudge2 == 1 )
		{
			*char_x -= RUN;
		}
	}
	/*他の方向への移動も同様に行なっています*/
	/*左移動のとき*/
	if(Key[KEY_INPUT_LEFT]>=1 && *char_x>0)
	{
		*char_x -= RUN;
		//左上
		map_i_converty = int( *char_y/AREASIZE );
		map_j_convertx = int( *char_x/AREASIZE );
		hitjudge1 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );
		//左下
		map_i_converty = int( (*char_y+squareside)/AREASIZE );
		map_j_convertx = int( *char_x/AREASIZE );
		hitjudge2 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );

		if( hitjudge1 == 1 || hitjudge2 == 1 )
		{ 
			*char_x += RUN; 
		}
	}
	/*下移動の時*/
	if(Key[KEY_INPUT_DOWN]>=1 && *char_y<MAX_Y)
	{
		*char_y += RUN;
		//左下
		map_i_converty = int( (*char_y+squareside)/AREASIZE );
		map_j_convertx = int( (*char_x+squareside)/AREASIZE );
		hitjudge1 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );
		//右下
		map_i_converty = int( (*char_y+squareside)/AREASIZE );
		map_j_convertx = int( *char_x/AREASIZE );
		hitjudge2 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );

		if( hitjudge1 == 1 || hitjudge2 == 1 )
		{ 
			*char_y -= RUN; 
		}
	}
	/*上移動のとき*/
	if(Key[KEY_INPUT_UP]>=1 && *char_y>0)
	{
		*char_y -= RUN;
		//左上
		map_i_converty = int( *char_y/AREASIZE );
		map_j_convertx = int( *char_x/AREASIZE );
		hitjudge1 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );
		//右上
		map_i_converty = int( *char_y/AREASIZE );
		map_j_convertx = int( (*char_x+squareside)/AREASIZE );
		hitjudge2 = (*MAP).Mapeffect( &map_i_converty,&map_j_convertx );

		if( hitjudge1 == 1 || hitjudge2 == 1 )
		{
			*char_y += RUN; 
		}
	}
}

どうでしょうか？
確かに、言われた通りプログラムをすっきりさせるように心がけると、プログラムが格段に見やすくなりました。
そして、当たり判定も正しく判定されるようになったので良かったです。

今までプログラムを綺麗に書くということは意識したことなかったので、汚いコードになり易かったのですが、
これからは意識して書いていこうと思います。
アドバイス有難う御座いました。

[softya（ソフト屋)]

大分見やすくなりましたね。
整理するのは自分の為になります。特に半年経つと意味不明のプログラムになるのを防げます。
それに今回のようにバグを混在させやすくなるので良いことは何もありません。

移動関連の処理は、更に関数に分けるともっとスッキリします。ほとんど同じ処理ですから。
９０行近くありますが、半分以下の行数になるのでは？

[Thelly]

そうですね、今日は今からレポート制作をするので時間がありませんが、時間ができ次第関数化させていきたいと思います。