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

Shape Collision data ordering #41

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

MondayHopscotch
Copy link
Contributor

I have run into a bug the previous two game jams I've participated in where the Array<CollisionData> passed into the listener callbacks was flipped from the order that a and b are passed in. More concretely, this is happening:

sa matches body a: false
sb matches body b: false

This PR fixes ordering of shape collision data so listener data matches order of bodies by properly passing the flip value through the various collide* functions.

@MondayHopscotch
Copy link
Contributor Author

The build failure here appears to be in completely unrelated heaps code:

/opt/hostedtoolcache/haxe/4.2.4/x64/lib/heaps/git/h3d/impl/ShaderCache.hx:14: characters 33-34 : Unexpected ?
Error: Process completed with exit code 1.

It doesn't seem like anything related to my code changes. Is this something that's happened before for PRs against this repo?

@MondayHopscotch
Copy link
Contributor Author

MondayHopscotch commented Oct 28, 2023

I think something in heaps changed. I noticed the workflow is just using git for heaps (which I know is generally recommended due to how heaps releases things) -- However, I changed to just use their latest release 1.10.0 from February, and the workflow seems to work:

Here is the workflow change

Here is the worfklow run passing using that version of heaps

I made a PR to propose these changes to the repo here, if this is desirable.

@AustinEast AustinEast merged commit 77e0f4f into AustinEast:master Jan 10, 2024
1 check failed
@MondayHopscotch
Copy link
Contributor Author

Does this still need to be reverted? Glad to help out if there are other issues outside of the ones related to the other PR. (also, welcome back!)

AustinEast added a commit that referenced this pull request Jan 10, 2024
…ata"

This reverts commit 77e0f4f, reversing
changes made to 3ca8cc9.
@AustinEast
Copy link
Owner

Yep, turns out it needed to be reverted! I started seeing this issue in the sample:
echo issue

@MondayHopscotch
Copy link
Contributor Author

😂 OK, that seems problematic. I'll take a look at the samples and try to come at my fix again. I'm a bit surprised that sort of behavior came out of my change, not gunna lie. I'll put up a new PR if I figure things out.

@AustinEast
Copy link
Owner

could you add a sample here that reproduces the issue you're seeing btw? i wouldnt mind looking over the issue

@MondayHopscotch
Copy link
Contributor Author

I meant to do that originally, but was using a game jam I was participating at the time as my test bed. Let me see if I can put together a minimal example to show the problem.

@MondayHopscotch
Copy link
Contributor Author

MondayHopscotch commented Jan 12, 2024

@AustinEast I made an example as a state in my fork here: https://github.com/MondayHopscotch/echo/tree/listener-shape-issue

I'm just tracing out the shapes as passed to the listener func and comparing them with the data. Note that the data doesn't match the shapes. More specifically, the data.sa and data.sb are flipped from the bodies a and b passed into the listener function, i.e. data.sa is the shape from b and data.sb is the shape from a.

This PR was attempting to set the flip flags correctly, but I clearly missed something. It looks like the problem manifests itself slightly differently for squares/polygons/circles. I'll keep tinkering and try to get to the bottom of it.

@MondayHopscotch
Copy link
Contributor Author

I've pushed up another commit that gets things half way to working (at least based on the samples). I fixed the data shape portion of it, and the physics craziness seems to be isolated to circles now. States without circles behave correctly. In that branch I also changed the optimized statics state to use static rects instead to test my theory. Getting closer. It looks like it's just a matter of the normals being wrong at this point (inverted from what is correct, resulting in a collision resolution that keeps throwing the shapes back and forth).

@MondayHopscotch
Copy link
Contributor Author

@AustinEast I put up a draft PR from my branch that appears to have fixed the issues we saw in the sample states here: #45

Per the description in the PR, I pushed up my other 2 states along with at least one TODO. If things look good, I will put up a clean PR without all the cruft in it.

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.

None yet

2 participants