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

There should be a way to disable a VisibleOnScreenEnabler #9069

Closed
ForestGameDev opened this issue Feb 11, 2024 · 3 comments · Fixed by godotengine/godot#88403
Closed

There should be a way to disable a VisibleOnScreenEnabler #9069

ForestGameDev opened this issue Feb 11, 2024 · 3 comments · Fixed by godotengine/godot#88403
Milestone

Comments

@ForestGameDev
Copy link

ForestGameDev commented Feb 11, 2024

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 AThousandShips added topic:rendering breaks compat Proposal will inevitably break compatibility and removed breaks compat Proposal will inevitably break compatibility labels Feb 11, 2024
@AThousandShips
Copy link
Member

AThousandShips commented Feb 11, 2024

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
Copy link

Cammymoop commented Feb 14, 2024

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
Copy link

Mickeon commented Feb 16, 2024

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

  • Have a configuration warning in the editor, when the VisibleOnScreenEnabler is an empty path.
  • Allow enable_node_path to be empty without errors.

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

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

Successfully merging a pull request may close this issue.

5 participants