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 two problems with Area2D and remove_child() #7508

Merged
merged 1 commit into from
Mar 4, 2017

Conversation

pwnSquirrel
Copy link
Contributor

  • When one of two or more overlapping Area2Ds is removed with remove_child(), it doesn't try to report to the other one anymore
  • When overlappinng Area2Ds are removed woth remove_child(), _enter_tree and _exit_tree signals are now properly disconnected upon removal

Fixes #7507.
Since the master branch doesn't compile for me at the moment without reverting some earlier commits and due to the many bugs it has right now, testing it is a bit difficult (but it works fine on 2.2). It should be fine though since the changes are minimal.

Also removed some unnessecary empty lines from the file.

@pwnSquirrel pwnSquirrel changed the title Fixes two problems with Area2D and remove_child() Fix two problems with Area2D and remove_child() Jan 12, 2017
@pwnSquirrel pwnSquirrel force-pushed the area2d-fix branch 2 times, most recently from 07198a9 to 88d5f54 Compare January 12, 2017 16:38
@akien-mga
Copy link
Member

Since the master branch doesn't compile for me at the moment without reverting some earlier commits and due to the many bugs it has right now, testing it is a bit difficult (but it works fine on 2.2). It should be fine though since the changes are minimal.

You probably just need to remove the .glsl.h files in drivers/gles3/shaders.

@pwnSquirrel
Copy link
Contributor Author

pwnSquirrel commented Jan 12, 2017

Thx, just figured that out.
It seems that the bugs don't occur in the master branch, although the file is basically unchanged. Could it be that connected methods aren't called anymore when their corresponding node is not inside the tree?

So I guess at least the first fix is obsolete, although I don't know why. Don't know about the second one though. I think it makes sense that the tree_entered and tree_exited signals are disconnected on any previously connected Area2D, if they are in the tree or not. They get reconnected anyway when the node joins the tree again.

@akien-mga
Copy link
Member

So if I understand correctly the change is no longer needed? Or if some part is still relevant, could you rebase to keep just that part? (also check the diff, you should not be altering the whitespace in parts that you are not modifying, and use tabs for indentation).

…e and _exit_tree signals are now properly disconnected upon removal
@pwnSquirrel
Copy link
Contributor Author

It didn't seem to cause problems anymore in the 3.0 branch (at least it didn't give any error messages).

However, I'm quite sure that the small fix that is left now is the way it should be. If an area2d is removed from the scene every connection to other area2ds and bodys should be cleared, even if those aren'T in the tree anymore. They will get reconnected when they are reentering anyway. If you remove overlapping connected area2ds at the same time, one of them will always be not in the tree anymore when the cleaning method runs, which causes an error in 2.1 / 2.2. And should also in 3.0, I don't know why it doesn't. Thats why I think this small fix is still relevant.

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

Successfully merging this pull request may close these issues.

2 participants