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 monitoring status of Area2D and doing same logic on Area too #8242

Merged
merged 2 commits into from
Apr 4, 2017

Conversation

volzhs
Copy link
Contributor

@volzhs volzhs commented Apr 3, 2017

Fix #8090

5b556ca makes monitoring always true when NOTIFICATION_ENTER_TREE
This PR fix regression but not to touch what 5b556ca tried to fix.

@volzhs volzhs added this to the 2.1 milestone Apr 3, 2017
@reduz
Copy link
Member

reduz commented Apr 3, 2017 via email

@volzhs
Copy link
Contributor Author

volzhs commented Apr 3, 2017

@reduz @akien-mga
Note that master branch or Area(3D) doesn't have 5b556ca commit. hm...
should we add same codes with 5b556ca on Area(3D) and master also?

@akien-mga
Copy link
Member

It seemed in #7508 that the fix was not necessary for the master branch, only the first part of it was merged. But @LoneSurvivor would likely know better than I do :)

@volzhs
Copy link
Contributor Author

volzhs commented Apr 3, 2017

@akien-mga what about Area(3D) on 2.1 branch?
#7508 fixed only Area2D but Area(3D) has same logic as @reduz said.

@akien-mga
Copy link
Member

I suppose a similar fix should be done for Area, though I'm not sure, I haven't really tried to reproduce the issue @LoneSurvivor was trying to address.

@volzhs volzhs changed the title Fix monitoring status of Area2D Fix monitoring status of Area2D and doing same logic on Area too Apr 4, 2017
@akien-mga akien-mga merged commit e10e732 into godotengine:2.1 Apr 4, 2017
@RandomShaper
Copy link
Member

RandomShaper commented Apr 6, 2017

Please note this is a breaking change!

I've builr with the latest sources and while running my game I've noticed my characters were not reacting. After some research, I've found this PR.

Not sure it it has something to do with this, but monitoring is setup as not to be stored if set to true.

UPDATE: monitoring is defined with ADD_PROPERTYNO which in turn sets PROPERTY_USAGE_STORE_IF_NONONE for it.

@volzhs volzhs deleted the area-monitoring branch April 6, 2017 19:46
@RandomShaper
Copy link
Member

Well, I'm a bit puzzled here. Probably the original issue had something to do with #7978.

Hopefully, the current code shows the correct behavior. I'll enable monitoring on my Area2Ds and check from now on if everything works well.

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.

4 participants