当たり判定クラスの作り方が知りたい。

Question

現状　[ 使い方はプレイヤークラス等のこのCollisionクラス変数を作ってプレイヤーの座標の変数のアドレスを変数に設定して
player->mCollision(enemy->mCollision);とすることで当たり判定の判定を隠してシンプルに位置を修正できます。 ]

問題　「　例えばenemy をベクターにした場合コピーコンストラクタでコピーされるためエネミー座標のアドレスを格納している変数が無効なアドレスになっています。現状これをUpdate();関数で毎フレーム設定することで修正しますがあまりい修正方法とは思えません。　」

やりたいこと「　どうすれば実装を隠した状態でやつコピーコンストラクタが動いても正常に座標を修正出来るのか知りたい。　」

Github: https://github.com/Shigurechan/Dungeon_Game/tree/43ddb23595d0b3498279c1b5c2acc2028521333b

	//背景を初期化
	for (int y = 0; y < STAGE_SIZE_HEIGHT; y++)
	{
		for (int x = 0; x < STAGE_SIZE_WIDTH; x++)
		{
			byte b = backStageBin.at(y * STAGE_SIZE_WIDTH + x);
			//printf("%x\n",b);
			switch (b)
			{
				//wall
			case (int)BinType::Wall:
			{	
				//std::cout << "あああ" << std::endl;
				backStage->push_back(MapObject(windowContext, glm::vec2(x * CELL, y * CELL), uvSize.at((int)b), MapObject::ObjectType::Wall));
				backStage->back().setPosition(glm::vec2(x * CELL, y * CELL));
			}
			break;

			
				//
			case (int)BinType::Ground_a:
			{
				//std::cout << "あああ" << std::endl;
				backStage->push_back(MapObject(windowContext, glm::vec2(x * CELL, y * CELL), uvSize.at((int)b), MapObject::ObjectType::Ground));
				backStage->back().setPosition(glm::vec2(x * CELL, y * CELL));

			}
			break;


				//
			case (int)BinType::Ground_b:
			{
				//std::cout << "あああ" << std::endl;
				backStage->push_back(MapObject(windowContext, glm::vec2(x * CELL, y * CELL), uvSize.at((int)b), MapObject::ObjectType::Ground));
				backStage->back().setPosition(glm::vec2(x * CELL, y * CELL));

			}
			break;
			}	
		}
	}

#include "Collision.hpp"

//コンストラクタ
Collision::Collision(glm::vec2 s, bool frag)
{
	size = s;
	position = NULL;
	isTrigger = frag;
	speed = glm::vec2(0,0);

}

//判定を移動
void Collision::setPosition(glm::vec2 *pos)
{
	position = pos;
}

//判定サイズを変更
void Collision::setSize(glm::vec2 s)
{
	size = s;
}

//トリガータイプを変更
void Collision::TriggerType(bool frag)
{
	isTrigger = frag;
}


//当たり判定
bool Collision::Intersect(std::shared_ptr<Collision> col)
{

		//std::cout << col.getSize().x << std::endl;
		//std::cout <<"ああああ:   "<< col->getPosition().x << std::endl;

	if (((col->getPosition().x + col->getSize().x) > position->x) && (col->getPosition().x < (position->x + size.x)) &&
		((col->getPosition().y + col->getSize().y) > position->y) && (col->getPosition().y < (position->y + size.y)))
	{
		std::cout << "Collision !" << std::endl;
		
		if (col->getTriggerType() == false)
		{
			glm::vec2 pos = *position + glm::vec2(CELL / 2, CELL / 2);					//自分
			glm::vec2 opponent = col->getPosition() + glm::vec2(CELL / 2, CELL / 2);		//相手

			const float vecX = pos.x - opponent.x;	//負の値なら自分が左側
			const float vecY = pos.y - opponent.y;	//正の値なら自分が下側

			//めり込み量の大きい方を補正する
			if (std::abs(vecX) < std::abs(vecY))
			{
				if (vecY > 0)
				{
					position->y = col->getPosition().y + col->getSize().y;
				}
				else if (vecY < 0)
				{
					position->y = col->getPosition().y - size.y;
				}
			}
			else
			{
				if (vecX > 0)
				{
					position->x = col->getPosition().x + col->getSize().x;
				}
				else if (vecX < 0)
				{
					position->x = col->getPosition().x - size.x;
				}
			}

			return true;
		}
		else 
		{
			return true;
		}
	}
	else {
		return false;
	}
}


//座標を取得
glm::vec2 Collision::getPosition()
{
	return *position;
}

//サイズを取得
glm::vec2 Collision::getSize()
{
	return size;
}

//トリガータイプを取得
bool Collision::getTriggerType()
{
	return isTrigger;
}

//移動速度設定
void Collision::setSpeed(glm::vec2 s)
{
	speed = s;
}

//移動速度取得
glm::vec2 Collision::getSpeed()
{
	return speed;
}



//デストラクタ
Collision::~Collision()
{

}

Comments

[退会済みユーザー]

文章を修正しました。

[退会済みユーザー]

> 現状　[]
> 問題　「」
> やりたいこと「」

上記のように括弧書きするぐらいなら、改行で区切るかMarkdownを使って書いてください

Answer 1

回答へのコメントも見た感じでは「ヒットした位置も知りたいからpositionをポインタ型変数にしている」といったように解釈しました。
ただ、それだけであればCollisionクラスにhitPositionといった変数を別で生やすなりすればいいわけで、わざわざpositionを危険の多いポインタ型変数にする理由がないです。

軽くエスパーしてみますが、もしかして
MapObject::setPosition
で、受け取ったglm::vec2型の引数に&をつけてCollision::setPositionにわたしているのでは？
この場合引数の値は意図しているほど長生きしないので、参照するタイミング次第では不正なアクセスとなり得ます。

Comments

[S.Percentage]

回答したあとに質問がガッツリ修正されているのでコメントで追記しますが、positionの操作もCollisionモジュールの責務とするのは範囲が広すぎるのでやめたほうが良いかと思います。
positionはMapObject(やPlayer？)が管理し、Collisionの結果によってMapObject自身がpositionの修正を適用するかどうかを判断するようにしたほうが自然です。
なんでもかんでも隠蔽するのが正義というわけではないので、モジュールがやるべきことをもう一度考え直して設計し直すのをおすすめします。

[fana]

見た感じ，Collisionというクラス自体に存在意義がなさそうなので，
衝突判定（と補正結果値の算出）に必要なデータを２つ受け取って，結果を返すだけの「関数」に挿げ替えるので事足りるように思いますね．

例えば，当たり判定が AABB であれば，
AABBデータを２つ受け取って，
衝突の有無と，衝突した場合の位置補正のためのオフセット量　を返す，みたいな．

Answer 2

話が全体的に意味不明なのですが，

Collision型のメンバpositionがポインタ型であることが問題を引き起こすという話なのであれば，
size やら speed とかと同じように，ふつーに glm::vec2 型にすれば根本的な問題を消滅せしむることができるんじゃないすか？　とか思うのですが，どうですか？

position だけをポインタ型にする必要性があるようにも見えないのですが．

Comments

[退会済みユーザー]

そうなのですがそうすると。どこで当たったものの座標を更新するのかわからなくなるのでポインタを使わざるおえないのです。

[fana]

全くおっしゃっている意味がわからないです．

そもそも　その Collision なる型が
なんのために，いつ時点で必要になって，どこまで生存してなきゃならないのか？
とか，諸々の話は全てあなたしか知らないので，妥当な解決方法とかも判断つきませんし．

例えば， Collision::Intersect() という処理が Collision 型の主たる存在意義なのだとしたら，
Collision::Intersect() を実施するときだけ存在していれば十分かもしれない．
であれば，「必要なときに作って Intersect() を実行して，そこで破棄される」形で使うならば，「コピーコンストラクタがどうの」という問題自体が起きないかもしれない．

Answer 3

Collision::positionをポインタでなく、実体を保持するようにすればよいのではないでしょうか。

               backStage->back().setPosition(glm::vec2(x * CELL, y * CELL));

このsetPosition()がCollisionのメンバ関数ならスタック変数を渡しているのでスコープから抜けたときに
消滅してしまうのでCollisionのpositionは不定値になってしまいます。

Comments

[fana]

> このsetPosition()がCollisionのメンバ関数なら

ひとつ前の行を見れば，これは MapObject::setPosition() の呼び出しだと見えます．

そこから何がどう巡って Collision::setPosition() に行き着くのか（あるいは行き着かないのか）は全く提示されていません．

[fana]

これはエスパーぎみな想像ですが

> コピーコンストラクタが起きることで

とか書かれているので，MapObject::setPosition() は，引数に指定された値をメンバにコピーする形にでもなっていて，そのメンバのアドレスを Collision::setPosition() に渡している　とかじゃないでしょうか．

[tatamyiwathy]

なるほど。確かにその構造だとCollision::positionをポインタにする気持ちもわからなくなないですねー

[fana]

> 確かにその構造だとCollision::positionをポインタにする気持ちもわからなくなないですねー

（そこは同意できないっす）

[tatamyiwathy]

w