Clear monitoring in Area* when its space changes to invalid

[Rindbee]

Clear monitoring in area* so that they can work properly in the following NOTIFICATION_ENABLED. it can work properly when the space changes to valid again.

Change space in advance to prevent disabled areas from being queried again.

That is, if the area as a monitor is disabled, it will no longer be added to the query list. Cached maps information will be cleared later in NOTIFICATION_DISABLED. Behavior is similar to exiting an area, as opposed to NOTIFICATION_ENABLED (which is similar to entering an area).

Fixes #61420 (Wrong behaviors caused by pause).
Fixes #76219.
Fixes #70848.

[markdibarry]

Just tested and it seems to work correctly if the CharacterBody2D is higher in the tree order than the Area2D, but if it's lower, it still outputs the following error when the process_mode is switched from disabled:

[Rindbee]

Yes, that is another issue. I don't have an idea yet, and I have no plans to solve it here.

[Rindbee]

Um, thanks _clear_monitoring(), I forgot to check the existing method, I was stupid.

I think it’s ready for review.

[felbecker]

Tried this PR under Godot 4.1.2 and it fixed #76219.

[akien-mga]

CC @mihe to review implications on godot-jolt (and maybe whether the change makes sense upstream ;)).

[mihe]

With this PR I'm seeing some strange behavior when using any disable_mode that isn't DISABLE_MODE_REMOVE, which I guess makes sense, since DISABLE_MODE_REMOVE is the only mode that actually removes the area from the physics space, but this PR calls _clear_monitoring regardless of what mode is used.

Seeing as how _clear_monitoring is currently being called in Area*D::_notification largely just to reconcile with the change of space that's happening in CollisionObject*D::_notification, I feel like this PR should instead do the following:

1. Remove the current handling of NOTIFICATION_EXIT_TREE and NOTIFICATION_DISABLED from Area*D
2. Introduce a new virtual void CollisionObject*D::_space_changed(RID p_new_space) {}
3. Invoke _space_changed wherever the space is changed in CollisionObject*D
4. Override _space_changed in Area*D
5. Call _clear_monitoring from Area*D::_space_changed if the new space RID is invalid

It wouldn't do anything to address the question of what DISABLE_MODE_MAKE_STATIC or DISABLE_MODE_KEEP_ACTIVE should mean in terms of an Area*D emitting enter/exit signals, which was arguably broken before this PR as well, but it would at least fix the bugs I'm seeing with the other disable modes, and make things a bit less fragile in the process.

[Rindbee]

Um, disable_mode was forgotten.

Providing a callback function will make it easier. Thanks for the suggestion.

[YuriSizov]

There isn't enough time to test this for Godot 4.2, so I'm bumping it. We should try to get this tested and validated early in the 4.3 dev cycle. Testing by everyone who is currently affected by the bug in their project would be highly apprecaited!

[markdibarry]

Tested and it works.

[YuriSizov]

I pushed a small addition that fixes the error spam from #70848 and the second case of #61420. I believe we were simply missing a check before adding to the self list. This code was added like that over 10 years ago by reduz, so it's likely to just be a silly mistake. mass_properties_update_list (or rather inertia something list it was before) has the check, for instance. So adding one for the active_list makes sense to me.

There don't seem to be any issues caused by that and everything works as expected. And I validated that this PR fixes both cases from #61420 and now also fixes #70848. I trust testing above that it fixes #76219.

The changes seem sensible to me and follow what @mihe suggested closely.

---

@markdibarry Is there anything else left that this PR doesn't fix? Something mentioned in comments, but not in the OP perhaps?

[markdibarry]

@YuriSizov Great job! I'll make a build and test this out today. The only thing nagging at me is that pausing/unpausing a scene causes enter/exit signals to retrigger in Area*D's. I could go 50/50 to guess whether or not that's intentional. It sounds like one of those things where both behaviors would be inherently problematic, and the onus is just unfortunately on the user to guard against. Either way, that's certainly outside the scope of this ticket, but if you have any insight on it, I'd appreciate it.

[markdibarry]

@YuriSizov Tested and can no longer reproduce either issue mentioned in #61420

[YuriSizov]

Thanks! Someone's daily counter will stop now 😛

[snakone]

Got this annoying error on godot 4.2.1 ""p_elem->_root" is true"

had to set process disabled to the other area aswell to solve it, and also

await get_tree().create_timer(0.2).timeout; process_mode = Node.PROCESS_MODE_INHERIT;