godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 69 forks source link

There should be a way to disable a VisibleOnScreenEnabler #9069

Closed ForestGameDev closed 7 months ago

ForestGameDev commented 7 months ago

Describe the project you are working on

A 2D sidescroller with many enemies with complex behavior in a big map

Describe the problem or limitation you are having in your project

I'm trying to use VisibleOnScreenEnabler2D to disable enemies out of screen, however using it causes issues when, for example, playing an animation where the enemy visibility "blinks", as soon as the enemy becomes invisible in the animation, the processing is disabled and hence the animation stops. Something similar happens when destroying the enemy, if I want to make it invisible and make something happen a few frames later, the process gets disabled as soon as the enemy is invisible.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I'd like VisibleOnScreenEnabler2D to honor the process_mode value, as it seems to ignore it. For this, this type of node should be created with process_mode = PROCESS_MODE_ALWAYS by default. So disabling its parent doesn't cause confusion to people.

If because of how it works internally, or in order to avoid a change in behavior for future versions, it must keep ignoring its process_mode, then adding a boolean or a method to pause its behavior temporarily would be enough. In this case, before I call my animations that make the enemy invisible I would set such boolean to TRUE.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

visible_on_screen_enabler_2d.pause = true -- run the animation -- await for my animation to complete visible_on_screen_enabler_2d.pause = false

If this enhancement will not be used often, can it be worked around with a few lines of script?

I was trying to use VisibleOnScreenNotifier2D to make my own implementation of a VisibleOnScreenEnabler2D that can be disabled, however I'm not sure how could I imitate its capabilities to use the render culling code to determine whether it's visible on screen. So I couldn't find any way to create a VisibleOnScreenEnabler2D that can be disabled.

Is there a reason why this should be core and not an add-on in the asset library?

It's a simple improvement on the existing VisibleOnScreenEnabler2D (and I guess VisibleOnScreenEnabler3D as well, however I haven't tested that one)

AThousandShips commented 7 months ago

Unless this is controlled by a Boolean option this would break compatibility, good to keep in mind, the process mode default shouldn't be changed because that also breaks compatibility

Cammymoop commented 7 months ago

For this use case this can easily be worked around by arranging the enemy's node structure so anything that needs to be hidden is not an ancestor of the VisibleOnScreenEnabler2D image

The VisibleOnScreenEnabler2D can effectively be disabled by setting it's enable_node_path to an empty string, though this will cause complaints in the log, setting it to some other node that doesn't have anything to do during process (such as itself) is also a potential albeit almost hilariously janky workaround. This proposal could be implemented just by not attempting to get the node if the path is empty, which would simply remove the complaints but not change functionality. EDIT: it would be more awkward to use (than a boolean toggle) when re-enabling the VisibleOnScreenEnabler2D but it already seems like a pretty niche use case so...

Also, the documentation for it's enable_mode property says

Determines how the target node is enabled. Corresponds to Node.ProcessMode. When the node is disabled, it always uses Node.PROCESS_MODE_DISABLED.

Which could be interpreted as the VisibleOnScreenEnabler2D always setting it's target to disabled if it itself is either disabled or has a process_mode explicitly set to PROCESS_MODE_DISABLED

I believe it's just intending to say that the "off" state it uses when it's not visible is always PROCESS_MODE_DISABLED

Mickeon commented 7 months ago

The error is rather silly. It feels like (probably is) an oversight. I think it would make sense to:

The behavior would match other things in the engine. It's not like AnimatedSprite explodes when sprite_frames is empty.