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

Improved CollisionNode's index validity check #421

Merged
merged 2 commits into from
Jun 26, 2015

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Jun 26, 2015

No description provided.

@jslee02 jslee02 added this to the Release DART 5.0 milestone Jun 26, 2015
@mxgrey
Copy link
Member

mxgrey commented Jun 26, 2015

Looks reasonable 👍

With the upcoming ShapeNode changes, we should probably think about how to automatically manage these relationships. One possibility might be to have a ShapeNode be aware of which other ShapeNodes its collisions are disabled with. We could have ShapeNode contain a std::unordered_set <ShapeNode*> (or something similar) that contains all the other ShapeNodes with which collisions are disabled (assuming we want collisions to be enabled by default).

With that scheme, the disabled collision relationships could be carried over when a Skeleton or World gets cloned.

@jslee02
Copy link
Member Author

jslee02 commented Jun 26, 2015

I agree that we should think about automatic management theses relationships. This is still workaround to prevent segfault due to invalid index. Also, we should consider how to make collision node is aware of property changes of associated ShapeNode such as size changing.

@karenliu
Copy link
Member

+1

jslee02 added a commit that referenced this pull request Jun 26, 2015
Improved CollisionNode's index validity check
@jslee02 jslee02 merged commit fd941e0 into release-5.0 Jun 26, 2015
@jslee02 jslee02 deleted the coll_node_index_check branch June 26, 2015 20:07
@jslee02 jslee02 modified the milestones: DART 5.0.1, Release DART 5.0 Jul 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants