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.

Sign up for GitHub

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

Jump to bottom

Fix multiple 2D & 3D physics issues (2.1) #8999

Merged
merged 1 commit into from
Jun 25, 2017
Merged

Fix multiple 2D & 3D physics issues (2.1) #8999

merged 1 commit into from
Jun 25, 2017

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented May 30, 2017 •
edited
Loading

  • Use NOTIFICATION_ENTER/EXIT_WORLD for Area (intead of *_TREE).
  • Now both bodies' and areas' constraints are cleaned up.
  • And now also that happens as soon as the space is set to null (i.e., when exiting the tree) instead of only at freeing time.
  • When clearing constraints, the loop goes on to the next if the current is already released, instead of breaking.
  • When one has been already released, no error is shown from now on, as it's something expected, since a pair (our kind of constraint of interest) can be freed by any of its involved collision objects and the other will try again.
  • Implement index shifting (or marking as -1) for shapes indices in collision pairs shapes are removed.
  • Standarize behavior of bodies and areas so that anything that invalidates a given pair gives the same result (collision mask, actual collision, etc); for instance, triggering area enter/exit signals.
  • Add missing member initializations.
  • Extend the new-space-equals-area/body-current-space test to every case.
  • Fix 3D ray-casts early accepting Areas (skipping the mask check).
  • Fix unpairing of large elements (2D's BroadPhase2DHashGrid).

Some of these prevent random crashes caused by constraints with dangling pointers to involved objects.
Fixes #8856.
Fixes #7589.
Fixes #6676.
And maybe others.

NOTE:
One of the issues is fixed by #9094, so I've worked on this with that one merged so it's a dependency.

UPDATE:
I've started noticing an issue that can make the engine crash. Not sure if it's something directly due to my modifications or if I have uncovered a latent bug somehow. So still researching.

UPDATE:
What I am being hit by is #6676. So I'm trying to hunt that bug and add a fix for it to this PR.

UPDATE:
Now I know objects are being properly destroyed at the C++ level (AKA, the #9094 debacle), I'm looking in other places.

UPDATE:
On my part, this is ready to review and test.
Also someone should check the eligibility of some or all of these fixes for master as I currently don't have the time to do it.

@RandomShaper RandomShaper changed the title [wip] Fix cleanup of constraints for 2D & 3D physics (2.1) Fix 2D & 3D physics (cleanup of constraints & more) (2.1) May 31, 2017
@RandomShaper RandomShaper changed the title Fix 2D & 3D physics (cleanup of constraints & more) (2.1) [wip] Fix 2D & 3D physics (cleanup of constraints & more) (2.1) May 31, 2017
@RandomShaper
Copy link
Member Author

RandomShaper commented Jun 1, 2017 •
edited
Loading

I've tested my game and some 2D & 3D offical demos involving physics, and everything seems to be working OK.

So, on my part, this is ready to merge for early adopters to test it a bit more.

@RandomShaper RandomShaper changed the title [wip] Fix 2D & 3D physics (cleanup of constraints & more) (2.1) Fix 2D & 3D physics (cleanup of constraints & more) (2.1) Jun 2, 2017
@RandomShaper RandomShaper mentioned this pull request Jun 2, 2017
Closed
@akien-mga akien-mga added this to the 2.1 milestone Jun 5, 2017
@RandomShaper RandomShaper mentioned this pull request Jun 7, 2017
Closed
@RandomShaper RandomShaper mentioned this pull request Jun 9, 2017
Closed
@RandomShaper RandomShaper changed the title Fix 2D & 3D physics (cleanup of constraints & more) (2.1) Fix multiple 2D & 3D physics issues (2.1) Jun 12, 2017
@RandomShaper RandomShaper changed the title Fix multiple 2D & 3D physics issues (2.1) [WIP] Fix multiple 2D & 3D physics issues (2.1) Jun 15, 2017
@RandomShaper RandomShaper changed the title [WIP] Fix multiple 2D & 3D physics issues (2.1) Fix multiple 2D & 3D physics issues (2.1) Jun 17, 2017
@RandomShaper
Copy link
Member Author

@akien-mga, please have a look at my last 'update' in this PR's description.

@RandomShaper
Copy link
Member Author

By the way, any chance for this to make it into 2.1.4?

If it's too late, it would be nice to have there at least the most straightforward fixes. I could make a separate PR out of them.

@akien-mga
Copy link
Member

I wanted to merge it for testing yesterday but you had tagged it as WIP so I refrained :) I'll try to have a cursory look at the current state and merge it.

@RandomShaper
Copy link
Member Author

RandomShaper commented Jun 19, 2017 via email

@RandomShaper
Fix multiple issues with 2D & 3D physics
3e5e8b6 
- Use `NOTIFICATION_ENTER`/`EXIT_WORLD` for `Area` (intead of `*_TREE`).
- Now both bodies' and areas' constraints are cleaned up.
- And now also that happens as soon as the space is set to null (i.e., when exiting the tree) instead of only at freeing time.
- When clearing constraints, the loop goes on to the next if the current is already released, instead of breaking.
- When one has been already released, no error is shown from now on, as it's something expected, since a pair (our kind of constraint of interest) can be freed by any of its involved collision objects and the other will try again.
- Implement index shifting (or marking as -1) for shapes indices in collision pairs shapes are removed.
- Standarize behavior of bodies and areas so that anything that invalidates a given pair gives the same result (collision mask, actual collision, etc); for instance, triggering area enter/exit signals.
- Add missing member initializations.
- Extend the new-space-equals-area/body-current-space test to every case.
- Fix 3D ray-casts early accepting Areas (skipping the mask check).
- Fix unpairing of large elements (2D's `BroadPhase2DHashGrid`).

Some of these prevent random crashes caused by constraints with dangling pointers to involved objects.
Fixes godotengine#8856.
Fixes godotengine#7589.
Fixes godotengine#6676.
And maybe others.
@RandomShaper
Copy link
Member Author

Sorry for the annoyance. The thing is that I've been auditing the code and I think everything should go together.

@akien-mga
Copy link
Member

I can't really assess all changes, but let's give it a spin and see how it goes.

@akien-mga akien-mga merged commit 148a9e3 into godotengine:2.1 Jun 25, 2017
@akien-mga akien-mga mentioned this pull request Jun 26, 2017
Closed
@akien-mga akien-mga mentioned this pull request Jul 2, 2017
Closed
@RandomShaper RandomShaper deleted the fix-physics-2.1 branch July 5, 2017 08:53
RandomShaper added a commit to RandomShaper/godot that referenced this pull request Jul 5, 2017
@RandomShaper
Fix initial collision pairs being cleared
a30dd84 
This affected both 2D & 3D physics and was a regression introduced by my
physics mega-fix (godotengine#8999).
@RandomShaper RandomShaper mentioned this pull request Jul 5, 2017
Merged
@RandomShaper
Copy link
Member Author

@akien-mga, I think some of these are needed for 3.0, but now I'm too busy. (And not forgetting #9518, which fixed a regression.)

What can we do? Wait for me being not so busy (no estimated timeframe) or what?

@RandomShaper
Copy link
Member Author

No problem. It's not that much. I'm porting them myself.

This was referenced Jul 25, 2017
Merged
Closed
@RandomShaper RandomShaper mentioned this pull request Aug 31, 2017
Closed
@akien-mga akien-mga mentioned this pull request Oct 20, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug topic:physics
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.
2 participants
@RandomShaper @akien-mga