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

Fix collision detection #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix collision detection #24

wants to merge 2 commits into from

Conversation

wi-re
Copy link
Collaborator

@wi-re wi-re commented Dec 11, 2024

There was an issue in the collision detection code that seems to have been around for quite some time, i.e., even the previous template had the same bug (see the old collisionDetect.h function lines 225-241 that has been unchanged for seven years) as part of the SAT.

The bug manifests in collision points for vertices to faces being computed wrongly (edge-edge works) where the returned result appears to always be the closest vertex to the origin, which is true in some cases but in most it is not. This stems from the 'handleVertexToface' function that is given the vertices of one OBB (described via its transformation matrix) and the difference between the centers of masses of the objects. How this code is supposed to work, I am unsure.

A more correct implementation would be to take the collision normal (which we have access to anyways) and use this normal to project the distance between the vertices (not center of mass) of the colliding object with respect to the center of mass of the collided object and pick the vertex for which this projection is smallest. To implement this we need to change the signature of the function to now take the center of mass as argument, as well as the normal. Implementing the code then is straight forward.

Credit for finding this problem goves to a student https://github.com/TobiasEppacher

This student also developed a simple test case to debug this issue (not shared here as it would require sharing their exercise submission to debug with).

@wi-re wi-re self-assigned this Dec 11, 2024
@wi-re
Copy link
Collaborator Author

wi-re commented Dec 11, 2024

Note that this code still needs to be cleaned up a bit as this was implemented on top of the pathfinder branch and contains those changes as well.

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.

1 participant