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

Area_enter doesn't work after some load/instance/changetree #8856

Closed
Kadhel opened this issue May 22, 2017 · 26 comments
Closed

Area_enter doesn't work after some load/instance/changetree #8856

Kadhel opened this issue May 22, 2017 · 26 comments

Comments

@Kadhel
Copy link

Kadhel commented May 22, 2017

Operating system or device - Godot version:

  • Windows 7 Pro, Service Pack 1
  • 64 bits
  • Godot_v2.1.3-stable_win64

Issue description:
In 100% of the cases, the following problem happens :
After reloading/reinstanciate a scene a few times, the area_object in it aren't detected by area_enter anymore.

Steps to reproduce:

  • Create a scene 1
  • Create a scene 2
  • Create a scene 3
  • Add some area objects in the scene 3
  • Add scene 3 as child of scene 2
  • Add one "area" as character in scene 2, and make it move in direction of the "area_objects" of scene 3. - Add a self.connect("area_enter",self,"someFunc") on the character of scene 2.
  • Add a button on scene 1 to go to scene 2, and a button to scene 2 to go to scene 1.
  • Make some "get_tree().changeScene("res://scene1")" and "get_tree().changeScene("res://scene2")" to switch between scene1 and scene2 a couple time.
  • After this, some areas of scene 3 aren't detected anymore by the area_enter of the character.

PS: When it happens, it persists (it reset only when leaving the game and comeback). And it can be seen that the "Collision Pairs", in Physics3d, in the monitor of the debugger, are lower than normal.

Link to minimal example project:
https://github.com/Mbenoni1/GodotEngineTestProject

This is a minimalist version of my own project. To see the problem happens, just launch, click play, then move with the arrows of your keyboard to take some elements. It will after a short time automatically come back to menu. Click "play", etc, and see the bug happens after 2 to 4 cycle.

Thanks ! If you have any questions, don't hesitate !

@bojidar-bg
Copy link
Contributor

Is this valid in 2.1.2 or 2.1.1 as well?

@bojidar-bg bojidar-bg added this to the 2.1 milestone May 22, 2017
@Kadhel
Copy link
Author

Kadhel commented May 22, 2017

I know that the problem is here in the 2.1.2 as well.
I've never used 2.1.1
Do you want me to make the test in 2.1.1 as well ?

@bojidar-bg
Copy link
Contributor

@Kadhel no need, I just wanted to ensure that #8242 is not the culprit here (since it got merged in 2.1.3).

@RandomShaper
Copy link
Member

I'm having a look at this.

Weird enough, running the project with visible collision shapes makes it work always. @Kadhel, can you try and tell if you get the same result?

@eon-s
Copy link
Contributor

eon-s commented May 26, 2017

I have a project with this problem but it never happens when I'm looking for it, and the connection were made with the editor, not script.

The weird thing is the same game I made with Unity have exactly the same problem 🙃

@Kadhel
Copy link
Author

Kadhel commented May 29, 2017

@RandomShaper I've tried with visible collision shapes and I have the same result, some collisions don't work after a few changeScene.

@RandomShaper
Copy link
Member

RandomShaper commented May 29, 2017 via email

@eon-s
Copy link
Contributor

eon-s commented May 29, 2017

Tested on this project removing shapes scaling and changing to box shapes, making all monitored and monitoring, and fail, even raycasts ignore the broken areas.

All shapes are there, like in my project.

@RandomShaper
Copy link
Member

RandomShaper commented May 30, 2017

With that PR merged, your hero is having his doses of coffee flawlessly. ;)

It's not finished work, but you can give it a try and tell me the outcome.

@Kadhel
Copy link
Author

Kadhel commented May 31, 2017

@RandomShaper praise

It seems amazing, but how can I test it until it got merged in 2.1 ? (sorry for being a little unexperienced about this xD)

@RandomShaper
Copy link
Member

@RandomShaper
Copy link
Member

If you need help putting that into practice, don't hesitate to ask.

@eon-s
Copy link
Contributor

eon-s commented May 31, 2017

@RandomShaper Still happens with your fix :S

@RandomShaper
Copy link
Member

Hmmm… I have more stuff merged in my local fork; maybe there is some beneficial interaction somewhere.

I'll check.

Anyway, I'm about to push some more changes to this PR. If anyone wants to test again, I'd be grateful since it's touching a lot of physics stuff.

@RandomShaper
Copy link
Member

What is actually making collisions work well is running the game on a debug build!

I'll keep researching and report.

@eon-s
Copy link
Contributor

eon-s commented May 31, 2017

@RandomShaper is possible, I think that made it with debug-release

@RandomShaper
Copy link
Member

Fresh news: on debug release, if I recompile without the 'DEBUG_MEMORY_ENABLED' macro, the error happens again.

Pretty weird, isn't it?

Can anybody compare debug with and without that macro and tell me if I am becoming crazy?

I've tried to debug whether the RingBuffer stuff was somehow alleviating some out-of-bounds writes. I've added sentinels before and after every allocated block of memory, but everything looks OK in that regard.

@eon-s
Copy link
Contributor

eon-s commented Jun 1, 2017

Something to add, not sure if helps:
Using stable, the not moving areas seem to detect the moving one, only the moving area is the one that sometimes miss.


@Kadhel check if making the Elements the ones that check area_enter (instead of the Character), works.

@RandomShaper
Copy link
Member

RandomShaper commented Jun 1, 2017

Good news!

I isolated the part of DEBUG_MEMORY_ENABLED that was making the game run correctly: it was the zeromem() that it's being applied to memory blocks just before free(). Ironically, that is commented as catch more errors, but instead it was masking one.

Then, after commenting out the zeromem(), I started to look for some uninitialized data that was possibily getting random values from already-released memory blocks. With zeromem and enough luck they were being initialized to zero (a sensible default for most variables) and thus the bug was being masked.

Some debugging after, I noticed that some "pickables" were being undetected because of Area.set_monitorable() (the node) wasn't calling PhysicsServer.area_set_monitorable() in turn for them so that the physics could enable monitorability for the node's server-side counterpart (AreaSW).

And then I found it: Area.monitorable was the uninitialized data I was looking for. And when it turned out to be non-zero by default, Area.set_monitorable(true) would find the value being already true, doing then no more than early-exiting the method.

The last version pushed fixes this.

@eon-s
Copy link
Contributor

eon-s commented Jun 2, 2017

@RandomShaper still no luck here when compiling with release_debug (on Linux at least, I'm using your fork).

@bojidar-bg
Copy link
Contributor

@RandomShaper Maybe we should have a switch to make the zeromem() be memset(1)? 😃

@RandomShaper
Copy link
Member

@eon-s, sure you have the latest version? Please check Area's constructor (scene/3d, IIRC) for monitorable = false.

@bojidar-bg, locally I have code to initialize allocs and reallocs (the added size, in its case) with a code-configurable value. And to fill blocks just before they are freed with the same. Maybe I can submit it. Now, I wonder if that value should be a constant or change across time to produce random behavior making bugs of this kind more noticeable.

@RandomShaper
Copy link
Member

It seems I haven't pushed correctly. So you cannot possibly have the most recent!

As soon as I get back to my computer, I'll repush.

@RandomShaper
Copy link
Member

Yes, that was the case. :P Pushed!

@eon-s
Copy link
Contributor

eon-s commented Jun 2, 2017

@RandomShaper works! :godmode:
I'll check other issues later with this 😃

RandomShaper added a commit to RandomShaper/godot that referenced this issue Jun 19, 2017
- 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.
@akien-mga
Copy link
Member

Fixed by #8999.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants