Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactoring GetIsCollidedWith #670

Merged
merged 3 commits into from Feb 11, 2021
Merged

Conversation

wraikny
Copy link
Member

@wraikny wraikny commented Jan 26, 2021

Changes/変更内容

Collider周りのリファクタリングです。
全てのクラスの組み合わせについて b2TestOverlap を呼び出す処理が書かれていて酷かったので、抽象化しました。

ただし生ポインタを使ってしまっているので、もう少しうまい方法があれば教えてください。

Issue

@@ -40,6 +40,19 @@ void Collider::SetTransform(const Matrix44F& transform) {
rotation_ = r;
}

bool Collider::GetIsCollidedWith(std::shared_ptr<Collider> collider) { return GetIsCollidedWith_(collider); }
bool Collider::GetIsCollidedWith(std::shared_ptr<Collider> collider) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getなのでconstはつけれませんか?

float radius_;

bool GetIsCollidedWith_(std::shared_ptr<Collider> shape) override;
protected:
std::pair<b2Shape*, int32_t> GetB2Shapes() override { return {&shape_, 1}; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

綺麗やるなら内部的にShapeCollectionクラス等を用意してそれを返すようにしたほうがいい気がする

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ポインタに関してはunique_ptrでもつとかしか手段思いつかないですね...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ただの値の場合とstd::vectorの場合を同一視している荒技なので、どうしたものか。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShapeCollectionというのは、このタプル相当のフィールドを持ったclassという感じの……?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ん?生ポなのがアレという話ではなく?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vector返す変わる的な何か

core/src/Physics/Collider/Collider.h Outdated Show resolved Hide resolved
float radius_;

bool GetIsCollidedWith_(std::shared_ptr<Collider> shape) override;
protected:
std::pair<b2Shape*, int32_t> GetB2Shapes() override { return {&shape_, 1}; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ん?生ポなのがアレという話ではなく?

@wraikny wraikny force-pushed the refactoring-get-is-collided-with branch from b98d6fb to 9d32535 Compare February 9, 2021 09:48
@durswd durswd self-requested a review February 9, 2021 15:28
@wistnki wistnki merged commit 990d0db into master Feb 11, 2021
@wistnki wistnki deleted the refactoring-get-is-collided-with branch February 11, 2021 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants