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

Add a shortcut to disable/deactivate a node in the editor #7715

Open
JaimieVos opened this issue Sep 17, 2023 · 87 comments · May be fixed by godotengine/godot#92377
Open

Add a shortcut to disable/deactivate a node in the editor #7715

JaimieVos opened this issue Sep 17, 2023 · 87 comments · May be fixed by godotengine/godot#92377

Comments

@JaimieVos
Copy link

Describe the project you are working on

When developing a game sometimes I want to set a node and it's children to an inactive state so I can test better without having to delete the entire node and losing track of what I removed.

Describe the problem or limitation you are having in your project

I cannot disable/deactivate nodes in the editor right now. This causes me to lose track of which nodes I deleted when I am testing something and I want a node to be gone for testing.

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

If I can disable/deactivate a node I can visually still see the node in the scene view. Then I have a clear view of which nodes are active and which are not.

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

In the inspector view there will be a disable/deactivate checkbox. If this is checked the node becomes faded or another color in the scene view.

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

I think it will be used quite often.

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

I think it is quite an essential part of being able to test.

@Calinou
Copy link
Member

Calinou commented Sep 17, 2023

This should be possible already by setting the node's process mode to Disabled in the inspector.

If you wish to hide a node in the editor or running project only, see #3433.

@RobProductions
Copy link

Thanks for the info @Calinou ! I tried it out and it works like you explained, but a really minor gripe would just be that they are 2 separate processes when OP's suggestion would unify it under a single click which would be faster/more intuitive for testing. I think it's something a lot of us from Unity will miss so I'm wondering if something like this would work:

image

Or even replacing the eye icon or switching it when you use a modifier key could be another option. And then basically when this new "enabled" state is on, it would override process to be disabled and visible to off. Or a potentially less destructive/easier approach would be to have this state inherit from Node and show up in the inspector like this:

image

But without the "metadata" label and maybe pushed to the top above "process". That's just an idea though so feel free to suggest any better options!

@markdibarry
Copy link

markdibarry commented Sep 17, 2023

This should be possible already by setting the node's process mode to Disabled in the inspector.

If you wish to hide a node in the editor or running project only, see #3433.

@Calinou The Disabled process_mode option currently breaks Area2D/Area3D nodes, but Rindbee just opened a PR to fix half of the problem. That being said, once it's fixed, it should cover this request.

@JaimieVos
Copy link
Author

Thanks for the info @Calinou ! I tried it out and it works like you explained, but a really minor gripe would just be that they are 2 separate processes when OP's suggestion would unify it under a single click which would be faster/more intuitive for testing. I think it's something a lot of us from Unity will miss so I'm wondering if something like this would work:

image

Or even replacing the eye icon or switching it when you use a modifier key could be another option. And then basically when this new "enabled" state is on, it would override process to be disabled and visible to off. Or a potentially less destructive/easier approach would be to have this state inherit from Node and show up in the inspector like this:

image

But without the "metadata" label and maybe pushed to the top above "process". That's just an idea though so feel free to suggest any better options!

Yes exactly, I agree with @RobProductions

@falquinho
Copy link

The functionality is there so we are talking just usability right?
I dont agree with adding a checkbox on the Scene panel, too much clutter.
Neither adding the "Enable" in the Inspector panel, that's duplication of concerns.
My suggestions:
1 - Add a project setting to not totally hide the Node when you click the Eye, but make it transparent or just an outline (con: you'd have to know this option exists);
2 - Add a "shortcut" to disable/enable process of a node in the right-click menu;
3 - Make the "Eye" a 3-state toggle: enabled, disabled and invisible, disabled but partially visible;

@RobProductions
Copy link

I dont agree with adding a checkbox on the Scene panel, too much clutter. Neither adding the "Enable" in the Inspector panel, that's duplication of concerns.

True, it's a bit of a repetition and maybe cluttering up the scene panel isn't the best. I'm just spitballing ideas, but also the fact that "hidden" is a concept only for "renderable nodes" (like 2D, 3D, etc.) makes it strange to prioritize it over "Node disabled" which every object inherits from.

Getting rid of the eye in favor of a "Node disable" button sounds drastic but to me makes more sense since now everything in the Scene hierarchy can use it and would also accomplish the same as the visible button, just with the added functionality of also disabling scripts. Then you could still edit "visibility" on the renderable nodes independently via the inspector panel. Again, just ideas though.

My suggestions: 1 - Add a project setting to not totally hide the Node when you click the Eye, but make it transparent or just an outline (con: you'd have to know this option exists); 2 - Add a "shortcut" to disable/enable process of a node in the right-click menu; 3 - Make the "Eye" a 3-state toggle: enabled, disabled and invisible, disabled but partially visible;

I like the third option as a good middle ground but I'm a little confused on what you mean by "partially visible". If you mean the setting toggles between 1. disabled (no script process and no visibility), 2. invisible (still processes) and 3. active (all on) then I'm definitely on board for that idea. There's still one other state though which is visible with no process, but at that point you could just use the process_mode to accomplish it.

This is getting complicated fast lol so I'll just say even though a new "disabled" property would repeat the functionality of process_mode, I'm still in favor of that because of its versatility and ease of use. Since it would exist at the highest level possible, it could be used as a universal parameter that affects rendering, scripts, and more; in my opinion, this is a pretty useful concept to speed up development without having to go through each descendant node disabling stuff.

@JaimieVos
Copy link
Author

The functionality is there so we are talking just usability right? I dont agree with adding a checkbox on the Scene panel, too much clutter. Neither adding the "Enable" in the Inspector panel, that's duplication of concerns. My suggestions: 1 - Add a project setting to not totally hide the Node when you click the Eye, but make it transparent or just an outline (con: you'd have to know this option exists); 2 - Add a "shortcut" to disable/enable process of a node in the right-click menu; 3 - Make the "Eye" a 3-state toggle: enabled, disabled and invisible, disabled but partially visible;

A shortcut would also be nice.

@Calinou Calinou changed the title Add a way to disable/deactivate a node in the editor Add a shortcut to disable/deactivate a node in the editor Sep 20, 2023
@inhalt120g
Copy link

I dont agree with adding a checkbox on the Scene panel, too much clutter.
Neither adding the "Enable" in the Inspector panel, that's duplication of concerns.
1 - Add a project setting to not totally hide the Node when you click the Eye, but make it transparent or just an outline (con: you'd have to know this option exists);
2 - Add a "shortcut" to disable/enable process of a node in the right-click menu;
3 - Make the "Eye" a 3-state toggle: enabled, disabled and invisible, disabled but partially visible;

I agree with everything except 3.
Right now, clicking on the eye can be used for quickly turning a, say, reference node on / off in order to confirm everything in the target node is as it should be. This is very useful when, for example, implementing interface where I often compare "reference" and "target" layers (first prepare a layout sketch in a graphic editing program to serve as a reference layer, export it as jpg/png and import it in Godot, make the menus… at this point there's lot of switching of node visibility on/ off, and later remove the reference). If the 3-stage eye icon were implemented, a simple turning visibility on / off would be very tedious.

An option I'd like to see is a simple MMB click on the eye to toggle it on/off. Or maybe Shift+LMB click or even just RMB click(whatever is the simplest). Reaction to RMB click is even already implemented (the interface reacts to it) but it just doesn't do anything.

I like how this is handled in Illustrator, where a layer can be visible / hidden / outline ("outline" is basically the equivalent of "disabled" here) and the eye icon has three states:

illustrator eye icon 3 states

@RobProductions
Copy link

If the 3-stage eye icon were implemented, a simple turning visibility on / off would be very tedious.

True, that's a very good point I didn't consider.

An option I'd like to see is a simple MMB click on the eye to toggle it on/off. Or maybe Shift+LMB click or even just RMB click(whatever is the simplest). Reaction to RMB click is even already implemented (the interface reacts to it) but it just doesn't do anything.

I like how this is handled in Illustrator, where a layer can be visible / hidden / outline

I like this idea too! To clarify it, you mean that the eye can simply show the "process disabled" status by utilizing transparency, right? If so, here's a potential mockup of the 4 possible states:

image

Where "Enabled" and "Disabled" reflect process_mode, and visible and invisible reflect what currently happens when you press the eye icon.

I think that definitely works as a quick way to show feedback from a shortcut button like you suggested. Unfortunately this would eliminate a current "feature" of the Godot UI which uses this "semi-transparent" style to show when a child of a node is invisible due to the parent visibility:

image

But imo it's worth it to be able to get more feedback about the state of the Node, and with the text changing transparency too it could work even for non-rendering nodes.

I'm still in the process of developing a game and learning about the Godot source so in about a month I'd like to start looking into the actual implementation of something that can address this. I do have a question to pose to the community though and that is: how people would feel about the proposed additional "Node disabled" property which would exist in the base Node class? The obvious downside is memory overhead but I still believe this would solve all the above concerns because it provides one quick action that lets you turn off everything related to the Node. To me that's a useful concept especially for things like level culling and debugging. The actual presentation of that button (whether it's a regular property, checkbox in the Inspector, or nested in a right click context menu) can definitely go any direction, but I'm wondering if other people find the tradeoff to be worth it? And would it be possible to prototype/mirror this behavior with a plugin?

@Zireael07
Copy link

Zireael07 commented Sep 25, 2023

Can't tell the difference between the four above?

Nvm I was looking at the wrong node

@RobProductions
Copy link

RobProductions commented Sep 25, 2023

Can't tell the difference between the four above?

Check the items at the bottom: CanvasLayer and RichTextLabel. Also I realized I misunderstood the idea from inhalt120g so I may do a second mockup to address it, sorry about that haha.

EDIT: Nevermind, I can't seem to make the idea make sense. I believe inhalt120g wanted to have "no eye" as an alternative look for the icon but sadly I don't think it'll mesh well because not all Nodes have the eye in the first place, only renderable ones. So you'd never be able to tell when that area is clickable or not. Instead I thought maybe the current "closed eye" could take its place but then the 4th state (disabled and invisible) has no distinguishable shape as you can see here:

image

And not to mention as a user I'd be confused on the functional difference between outlined eye and closed eye, and having 4 different shapes would definitely be overwhelming. Considering that, I'll stick by my original mockup from the other comment and propose that as my best user feedback idea for "disabled node". And of course my question from above about a new property still stands :)

@Zireael07
Copy link

Check the items at the bottom: CanvasLayer and RichTextLabel.

Ah, I was looking at the top. This idea makes sense, more than just varying the opacity of the entire icon <3

@inhalt120g
Copy link

Thinking about it, simply crossing out the disabled node (strikethrough) would probably be the best indicator of the status. Here is what it could look like:
image

Alternatively, how about a "pause" sign added to the icon?
image

@RobProductions
Copy link

RobProductions commented Sep 29, 2023

Thinking about it, simply crossing out the disabled node (strikethrough) would probably be the best indicator of the status.

Great ideas! But I think strikethrough is a bit too distracting when you need to clearly read the name of disabled objects. Remember that disabled nodes could exist anywhere and could be used for culling/game mechanics, and not just temporarily for debugging.

Alternatively, how about a "pause" sign added to the icon?

This is a lot more clear. I like this (especially if there's no extra "disabled" property implemented) since it can clearly show the process_mode status independently and would work well with the shortcuts mentioned above!

EDIT: One caveat though is that the eye is only shown for a subset of nodes, not all nodes, even though process_mode exists for all nodes. Definitely still nice to have but doesn't cover all bases. However, combined with the "transparency" idea I made above, it can still work since the text can fade without needing an icon. Just a thought haha

@inhalt120g
Copy link

inhalt120g commented Sep 30, 2023

Without taking the functional aspect into account I also prefer the pause sign, but in context I actually prefer the strikethrough because it's very obvious (deactivated node is quite a big deal). Also strikethrough is more newbie friendly (in the case of "pause" you really have to know where to look) so it'd help avoid situations where someone might forget to activate something back after deactivating it.

And in case nodes ever have to have something more added to them in the future (visually, like some indcators or whatever), strikethrough is so flexible that it'd allow it no problem. For example let's say in the future a small "warning" sign has to appear next to nodes where "pause" was shown in the previous sketch, there wouldn't be enough space for both markings.

strikethrough is a bit too distracting when you need to clearly read the name of disabled objects

If strikethrough is getting in the way of reading, hmm maybe it can be slightly transparent?

@RobProductions
Copy link

Also strikethrough is more newbie friendly (in the case of "pause" you really have to know where to look) so it'd help avoid situations where someone might forget to activate something back after deactivating it.

And in case nodes ever have to have something more added to them in the future (visually, like some indcators or whatever), strikethrough is so flexible that it'd allow it no problem. For example let's say in the future a small "warning" sign has to appear next to nodes where "pause" was shown in the previous sketch, there wouldn't be enough space for both markings.

True, but the same could be said about the "transparent" approach I tried above; if the text color simply fades out when disabled you would also get to see at a glance which nodes are disabled, but without visually punishing you as much for utilizing it since strikethrough to me feels like something you have get rid of or mark as complete. And it allows the same room for more icons in the future too.

Thinking about it, the fade out tactic is actually what Unity does for disabled GameObjects, and for good reason it seems lol.

If strikethrough is getting in the way of reading, hmm maybe it can be slightly transparent?

Okay! Let's try it out then:

image

Definitely better on readability but imo this is still too much visual clutter within the scene panel. Any less opaque than this and you start to miss the fact that the line exists at all. I'm also trying to consider the fact that for whatever reason someone's project could have every node disabled since it's meant to reflect script processing only. At that point we might as well just make another icon to show it but I don't think that was a popular idea either.

Sorry for having strong opinions on it haha but I still believe transparency can solve all of these issues because it is: clear at a glance, doesn't eat up space from other icons, preserves the eye icon functionality, and doesn't introduce more clutter because the "fade out" style is already used for the icons themselves. But definitely let me know if there are any other ideas or ways to address this stuff!

@OhiraKyou
Copy link

OhiraKyou commented Sep 30, 2023

Is actual, practical usage being considered here? Who is this theoretical person who needs to specifically toggle visibility without toggling processing? You generally have separate visual nodes (like mesh instances) to toggle anyway. Even for UI, there should be separate layout and rendering component nodes.

Even coming from Unity, where a single game object could have an arbitrary number of visual and functional scripts crammed in, I have never once felt the need for a visual toggle on the object level. Why? Because of composition; if I wanted to hide a mesh, I would toggle the mesh renderer component instead of the whole object. And, in Godot, composition is done through nodes. So, I would just toggle the mesh node.

If your visual and functional nodes are so tightly coupled that they require a separate node-level toggle specifically for visibility, you should probably just separate them. Then, you'll have your separate node-level toggle automatically, because they will be separate nodes.

Example

As an example, assume you have a mesh with collision and you want to be able to toggle visibility and collision separately. Instead of organizing your node like this:

  • MeshInstance3D
    • StaticBody3D
      • CollisionShape3D

Drag the static body out to a new parent, resulting in a structure like this instead:

  • Node3D
    • MeshInstance3D
    • StaticBody3D
      • CollisionShape3D

With that done, you now have separate visual and physical components and can toggle each one separately. And, you can still toggle the entire object by toggling the parent.

If you have multiple visual components, simply group them, like this:

  • Node3D
    • VisualComponent (Node or Node3D, depending on if you need the extra transform)
      • MainBodyMesh (MeshInstance3D)
      • AccessoryMesh (MeshInstance3D)
      • Particles
    • StaticBody3D
      • CollisionShape3D

Now, you can also either toggle the whole visual component or the individual pieces.

Suggestion

So, I would expect the toggle to both affect visibility and processing. And, I would expect the recommendation to be this: if you want the toggle to just hide an object, compose your object such that the visible components are separate in the first place. Your visible nodes shouldn't be "processing" anything other than visuals anyway.

Also, the existing behavior (toggling visibility only) can be left as a user setting. If separate buttons are used for toggling visibility, processing, and both at the same time, just one of those buttons can be included by default (ideally, the dual-purpose button). And, users could simply enable and disable any of those three in the settings.

Aside

As an aside, for intuition, consider the following: what's so special about visuals? Why not an object-level toggle for audio components? Or physics? Or lights? Or any other arbitrary category? How many states does the toggle icon need to support then?

@Zireael07
Copy link

Godot is not Unity - there are no component scripts. There are a lot of cases where you might want to make something invisible (especially in remote tree, so that something doesn't cover what you want to see at the moment), but keep it processing/active

@OhiraKyou
Copy link

OhiraKyou commented Oct 1, 2023

There are component scripts, because nodes are components, and they can have scripts; I'm using the term in the general sense (component as in "composition"). And, in the case you're describing, your "something" should be split into a visual component (node) and functional component (node). Then, a single, dual-purpose toggle would affect them both equally while enabling toggling visual and functional aspects of the composite object at will.

@RobProductions
Copy link

RobProductions commented Oct 1, 2023

If your visual and functional nodes are so tightly coupled that they require a separate node-level toggle specifically for visibility, you should probably just separate them. Then, you'll have your separate node-level toggle automatically, because they will be separate nodes.

So this is actually similar to the first idea I proposed which would add "disabled" as a property to the base node class, allowing every descendant Node and child node to benefit from it. I would actually favor the approach you described to how Godot currently works, but I also didn't want to propose drastic changes to the way users interact with the nodes currently. That's why I believe that simply adding this property first is a good step towards that goal because it opens up the use case and in the future when people have had a chance to adjust, we could consider removing the visibility toggle if the same is achievable through the node activation.

But speaking from personal opinion, I'm of the belief that more options are better and I would prefer both in the long run; a universal disabling property and a visibility property that only affects rendering. While you could technically achieve any combination with just the one through composition, having granular control over each part of the node seems useful too.

So, I would expect the toggle to both affect visibility and processing. And, I would expect the recommendation to be this: if you want the toggle to just hide an object, compose your object such that the visible components are separate in the first place. Your visible nodes shouldn't be "processing" anything other than visuals anyway.

The discussion veered into shortcuts because I wasn't sure if people wanted to eliminate the old behavior or rely on some new property. But there is one other complication that would make this shift more challenging: process_mode.

There are multiple states in process_mode, not just an on or off toggle. It can be running when paused or unpaused, disabled, inherited, etc. So if we add a universal "disabled" property, what happens to process_mode? Does it sit on top of process_mode? Or does process_mode become this new feature, and "disabled" also means "don't render"?

Again I will be in favor of granular control and propose that a new property could sit in the base node (separate to process_mode) which overrides process_mode when the property is set to "disabled". Then, much like visibility, in the future process_mode could maybe be reworked or removed if the same becomes achievable through some other method (like, say, a "paused" field you can read through code? I'm sure something like that already exists).

As an aside, for intuition, consider the following: what's so special about visuals? Why not an object-level toggle for audio components? Or physics? Or lights? Or any other arbitrary category? How many states does the toggle icon need to support then?

I'll again mention another suggestion I had above which is to remove the eye icon entirely and just have it be this new "disabled" property:

Getting rid of the eye in favor of a "Node disable" button sounds drastic but to me makes more sense since now everything in the Scene hierarchy can use it and would also accomplish the same as the visible button, just with the added functionality of also disabling scripts. Then you could still edit "visibility" on the renderable nodes independently via the inspector panel.

In this way I believe it should address your concern since the button becomes more universal and the individual component toggles can still exist within the node inspector panel if people need them. I didn't want to presume any specific direction though which is why I'd also settle for a shortcut/feedback UI for process_mode.

So to summarize: there seems to be 2 separate but related ideas in this thread. One is a shortcut/feedback UI for toggling the existing process_mode, and another is the addition of a new property that may or may not replace the existing state toggles. Then, if a new property was added, a further step could be taken to replace the eye icon/feedback UI to use it. Hopefully I got that right, and I'd appreciate any other suggestions or ideas to define the best direction here. In about a month or so after my current game is complete I'll probably start learning the Godot source and this is definitely something I'd like to look into.

@hsandt
Copy link

hsandt commented Oct 24, 2023

So to summarize: there seems to be 2 separate but related ideas in this thread. One is a shortcut/feedback UI for toggling the existing process_mode, and another is the addition of a new property that may or may not replace the existing state toggles. Then, if a new property was added, a further step could be taken to replace the eye icon/feedback UI to use it. Hopefully I got that right, and I'd appreciate any other suggestions or ideas to define the best direction here. In about a month or so after my current game is complete I'll probably start learning the Godot source and this is definitely something I'd like to look into.

Yes, I think that's it. However, note that the UI feedback will also be useful for the first idea: we need some feedback that we actually disabled a node, because currently we can only check this by scrolling the inspector to the bottom and check the Process Mode. Feedback could be checkbox, grey out, etc. as we've already researched above (with the subtlety that with the first idea, only one enum value DISABLED would have the feedback so there wouldn't be a 1:1 relationship between property and visual feedback, unlike with the second idea).

I think it'd be faster to start with the first idea: implementing a keyboard shortcut to toggle process mode between INHERIT and DISABLED + a visual feedback (e.g. gray out) + toggle visibility, which would work with 90% of the nodes. But I admit that the fact that the operation would be destructive for nodes in different process modes may disturb some users (e.g. if you two toggle twice an ALWAYS node, it will switch to DISABLED then INHERIT so you end up in a different mode).

For a concrete use case: if you wanted to temporarily disable a Pause Menu in ALWAYS process mode, you'd have to make sure to disable some parent node of it that originally had an INHERIT process mode to avoid losing information.

If the implementer feels like they can skip the first idea and implement the second idea with the new metadata enabled on the go, though, they can just go ahead!

@ratzycon
Copy link

ratzycon commented May 20, 2024

this seems bizarre, there is no simple disable node tree, should be fast, easy, 1 click
why does nodes not have a disabled state? the top function in nodegraphs should be 'if active:'
atm disabled nodes still run ready functions
a scenegraph based system is exactly supposed to have easy ways to disable subgraphs, and each base node should have a basic on/off switch to completely ignore the node, until it's state changes
similar to how Awake() will not be called on a deactivated object in unity

@Calinou
Copy link
Member

Calinou commented May 21, 2024

this seems bizarre, there is no simple disable node tree, should be fast, easy, 1 click

This exists since Godot 4.0:

image

@zydeas
Copy link

zydeas commented May 21, 2024

This exists since Godot 4.0:

Sorry to be pedantic, but that's three clicks: Click on node to select it, scroll down to bottom of inspector if it's too long, click on dropdown menu, click on bottom item. And as far as I remember, there's no easy way to tell if the node has processing disabled by just looking at it in the hierarchy.

@zydeas
Copy link

zydeas commented Jun 19, 2024

I see what you mean by your example. Here's an alternate one that I can come up with off the top of my head. What if your described level is really big and complicated and you want to completely deactivate chunks of it depending on where the player is to save on any potential performance (let's say you have enough complex stuff in your game that it actually does matter). And the button that happens to open the door is way across the map and in a place where the player can't see (assume that, for whatever reason, you don't actually show the door opening to the player. Maybe you have an onscreen message telling them a door opened and they have to find it).

So the door definitely needs to open when the player presses this button, but it also definitely won't be active when the player is standing by the button. In a case like that, I think you'd definitely want to be able to say "this signal should fire on this target thing, even if it's inactive". (Of course, you could just not have a signal and set up an actual function on the door for making it open, but that bypasses the whole point of signals.)

Aside from that, I'll add here: I think it may be a good idea to have the default be "inactive nodes can't receive them", but provide a way for the coder to override that. Either a toggle on the signal itself, a toggle on the whole node that lets every signal through (I'd add an argument to EmitSignal but that already uses a variable number of args for signal parameters).

@passivestar
Copy link

yeah that's an interesting example. but it seems like the existing toggles (visibility and processing) already solve your case perfectly:

  • You care about not rendering it for less GPU time
  • You care about not processing it for less CPU time
  • You would actually prefer the engine notifications to fire because they too rare to have any performance hit and you may need them to keep the object's state consistent (i.e you may need to do something when the node is parented/unparented, when children are added/removed, etc)
  • Since it's for performance it means you'll only be disabling it from code so you won't be benefiting from the convenience of having a nice single checkbox in the tree view. Writing 2 lines of code instead of 1 doesn't really matter in the context of the visibility manager that you'll need to write to hide faraway objects

Hope it makes sense

@Snowdrama
Copy link

Snowdrama commented Jun 19, 2024

tbh nothing else comes to mind atm, as long as inactive means that no code can be executed on a node including all notifications, input callbacks, etc I'd personally be happy with that

I don't necessarily agree we should be preventing the lifecycle calls like _EnterTree, _ExitTree, and _Ready when a node is disabled. To me they signify specific things that are still valid EVEN if the node is disabled. They should continue to fire even if the node is disabled because they have actually entered the tree, and are "readied" due to being loaded as part of a scene.

This is where the additional lifecycle function come in like _OnEnabled _OnDisabled and the _OnActiveChanged

If you previously did something in _EnterTree or _Ready you would switch it to _OnEnabled then you could mark it as disabled and then at some other point make it active. The initialization can then happen in _OnEnabled if it truly needs to happen when it's enabled like maybe it sends a signal. But you also get to frontload work like if you were making a procedural dungeon, and you wanted to generate the chunk of the map when it enters the tree, as part of the scene loading, and not when it becomes active. It can start disabled, but still do work as part of it entering the tree.

If we are to change it so _EnterTree, _ExitTree, and _Ready are not called, we would need to have a discussion about when in the lifecycle they are called, as well as the implications of lying to the parent telling that the children are _Ready even if a child was never readied or even entered the tree. Since the parent can have a reference to it, the triggering of the _Ready function of the parent signals that the children are initialized, even if they aren't?

@passivestar
Copy link

This is where the additional lifecycle function come in like _OnEnabled _OnDisabled and the _OnActiveChanged

I fully support adding those, yes. I don't even mind if enter/exit tree are executed if enable/disable are easily available because I can just never put any logic into those old lifecycle methods and only use _on_enabled _on_disabled. It's important though that _on_enabled is actually called at appropriate time when the node is first created (I know it's the case in Unity but I haven't checked in Godot yet)

@Snowdrama
Copy link

Snowdrama commented Jun 19, 2024

It's important though that _on_enabled is actually called at appropriate time when the node is first created (I know it's the case in Unity but I haven't checked in Godot yet)

Yeah for sure the idea would be that if it enabled in the serialized scene, then during scene loading the _on_enabled function would be called as part of the scene load lifecycle. Whether it comes before or after _ready is a question to be had and probably needs more investigation.

Similarly _on_disabled would be called along side _exit_tree during the process of removing a node when it's freed with queue_free

both _on_enable and _on_disable would be called just like current functions like _ready and _enter_tree called automatically when the state changes using something like myNode.active = false or myNode.set_active(true) and the hope would be that similarly to visibility it would also have a signal to listen for the state changed, like visibility_changed we'd have an on_active_changed

@RobProductions
Copy link

both _on_enable and _on_disable would be called just like current functions like _ready and _enter_tree called automatically when the state changes using something like myNode.active = false or myNode.set_active(true) and the hope would be that similarly to visibility it would also have a signal to listen for the state changed, like visibility_changed we'd have an on_active_changed

I like this idea, and on_active_changed is something I already implemented actually as you can see here:

image

Though in hindsight it should be exposed by Node not Node3D, it was just necessary there at first to help propagate visibility stuff. Will fix that when I can. Is this "active state callback" approach the best option then? From what I'm seeing it should be possible to create these new lifecycle calls for GDScript and should be less disruptive to the existing calls, but I just wanted to double check that everyone's workflow is satisfied by that!

One problem though is that in terms of naming enabled and disabled are already used internally:

image

These notifications were originally run by process_mode changes so that more specific nodes could run behaviors when processing is disabled, but the new active state stuff also runs them for this same purpose now. However, that means if I run the GDScript callbacks when these notifications are sent they also will be run when process_mode changes. I think we lose something with that approach because it removes the ability to get more granular with "disabled" vs "don't process" states if that makes sense, so I'm leaning towards keeping them separate from the existing notifications.

So if we want these new lifecycle calls to only be triggered by active state changes, it would make sense to use a more unique name. I can go with something like _on_active and _on_inactive for now but are there any other ideas/preferred naming? Or maybe we do want these calls to be called by process_mode changes? Let me know!

As a reminder this feature I'm building is meant to do two things:

  1. Act as a shortcut for debugging/culling through API and Editor UI (which is what this proposal was originally aimed at)
  2. Add a cleaner/more universal way to completely disable a node's impact in the scene in both editor and runtime

The first point definitely has a greater impact on my specific workflow so I appreciate opinions on the second point and hope to shape it to match expectations there. We've got some great ideas so far I think :)

@Snowdrama
Copy link

I like this idea, and on_active_changed is something I already implemented actually as you can see here
Though in hindsight it should be exposed by Node not Node3D, it was just necessary there at first to help propagate visibility stuff. Will fix that when I can.

Yeah sounds like you get the idea, I think on_active_changed would be on Node itself, but I could imagine having it change visibility on CanvasItem and Node3D could be a challenge so I get it haha

One problem though is that in terms of naming enabled and disabled are already used internally

Ooo I didn't know about that one, I'm just starting to dig into the engine side of things. So enable/disable are triggered when process is changed? Interesting.

I think we lose something with that approach because it removes the ability to get more granular with "disabled" vs "don't process" states if that makes sense, so I'm leaning towards keeping them separate from the existing notifications.

Absolutely, I do like the granularity around process_mode enabled/disabled vs a node's active/inactive state. I think the active/inactive triggering visiblility and process_mode callbacks is good for sure. I'd be curious to see how people think about what happens if I change the visibility or process_mode on an inactive node.

Does setting visibility/process_mode or anything like that would reactivate the node? Or we could throw an error like "Node is inactive, process_mode can't be changed while the node is inactive" like the active state overrides visibility, and process_mode.

Consider this:

@export var my_node : Node3D

func do_something():
    my_node.visible = true;
    my_node.node_active = false
    print("Is Node Visible?:", my_node.visible) # false? Did we change visible to false?
    
    my_node.visible = false;
    my_node.node_active = true
    print("Is Node Visible?:", my_node.visible) # what's the expectation here? is it forced visible?

If it's actually changing the value of visible under the hood, then we may be losing the state of the node before it was disabled leading to unintended effects when re-enabling.

I Personally think overriding the state makes the most sense to me as it preserves the state regardless of if it's active or inactive, but I want to see how other people feel. Here's an example of how that might look:

# my_node stays true while active, checking it while inactive returns false because it overrides the visibility state.
@export var my_node : Node3D
func do_something():
    my_node.visible = true
    print("Is Node Visible?:", my_node.visible) # true
    my_node.node_active = false
    print("Is Node Visible?:", my_node.visible) # false it's overridden
    my_node.node_active = true
    print("Is Node Visible?:", my_node.visible) # true because it was true before becoming inactive
    
# my_other_node stays false, setting it back to active doesn't change the visibility back to true
@export var my_other_node : Node3D
func do_something_else():
    my_other_node .visible = false
    print("Is Node Visible?:", my_other_node.visible) # false
    my_other_node.node_active = false
    print("Is Node Visible?:", my_other_node.visible) # false it's overridden
    my_other_node.node_active = true
    print("Is Node Visible?:", my_other_node.visible) # still false

Hopefully these examples help make what I'm getting at clear.

So if we want these new lifecycle calls to only be triggered by active state changes, it would make sense to use a more unique name. I can go with something like _on_active and _on_inactive

If NOTIFICATION_DISABLED is tied to the changing of process_mode maybe it might make sense as maybe another proposal/change to change they enum name to something like NOTIFICATION_PROCESS_MODE_DISABLED to clarify that it's the process_mode that's getting enabled/disabled.

I assume adding lifecycle callbacks for _on_active and _on_inactive would need notifications as well, so In the same vein as that though we could add "node" to it like NOTIFICATION_NODE_ACTIVE and NOTIFICATION_NODE_INACTIVE and maybe the lifecycle callbacks become _on_node_active and _on_node_inactive for clarity.

I do want to see if maybe someone else has any ideas besides active/inactive but I do think active/inactive makes sense here.

As a reminder this feature I'm building is meant to do two things:

  1. Act as a shortcut for debugging/culling through API and Editor UI (which is what this proposal was originally aimed at)
  2. Add a cleaner/more universal way to completely disable a node's impact in the scene in both editor and runtime

The first point definitely has a greater impact on my specific workflow so I appreciate opinions on the second point and hope to shape it to match expectations there. We've got some great ideas so far I think :)

I definitely get the use cases for debugging by isolating stuff by deactivating other things in a scene, but I think there's other benefits to other uses, like object pools, and tools and stuff so I'm glad there's a good discussion around it and I think there's a lot of really good ideas between everyone.

@RobProductions
Copy link

Ooo I didn't know about that one, I'm just starting to dig into the engine side of things. So enable/disable are triggered when process is changed? Interesting.

Yup but they are not exposed to GDScript, just "notifications" which help the nodes communicate in c++ world.

Does setting visibility/process_mode or anything like that would reactivate the node? Or we could throw an error like "Node is inactive, process_mode can't be changed while the node is inactive" like the active state overrides visibility, and process_mode.

Neither, the way I implemented this is that active state is simply another property like visibility that can be toggled on or off, without either of them throwing an error. In the code that checks whether a node should render, it simply checks both visibility and node_active, hence it acting like an override.

func do_something():
    my_node.visible = true;
    my_node.node_active = false
    print("Is Node Visible?:", my_node.visible) # false? Did we change visible to false?

Just tested this to be sure; so in this example my_node.visible is true because the "visibility field" is true. If you want to check whether or not the node is "currently visible in the scene" you could call my_node.is_visible_in_tree() which accounts for node_active and parent nodes that have visibility/active state set to false. This is basically what you have to do anyways currently in Godot if you want to check for parent node visibility so it should make sense to most users :) It's also the same as Unity if you've ever used it, where you can call SetActive() on an object that has it's renderer component set to disabled and basically get granular control both in the editor and in code for each toggle.

Similarly, as I mentioned above I've got is_node_active() to tell if it and all parents are active, and is_node_active_self() to tell if it is active individually. I can rename those if there's a better option :)

If NOTIFICATION_DISABLED is tied to the changing of process_mode maybe it might make sense as maybe another proposal/change to change they enum name to something like NOTIFICATION_PROCESS_MODE_DISABLED to clarify that it's the process_mode that's getting enabled/disabled.

I assume adding lifecycle callbacks for _on_active and _on_inactive would need notifications as well, so In the same vein as that though we could add "node" to it like NOTIFICATION_NODE_ACTIVE and NOTIFICATION_NODE_INACTIVE and maybe the lifecycle callbacks become _on_node_active and _on_node_inactive for clarity.

Strictly speaking I don't think it's necessary (so long as something runs the GDVIRTUAL_CALL() macro) but definitely good to have from what I can tell! I like these names so I'll go with that for now as I try my hand at implementing it

I'm glad there's a good discussion around it and I think there's a lot of really good ideas between everyone.

Absolutely, really happy to hear your thoughts on this stuff and make workflow improvements that hopefully help everyone in the long run :) Will report back when I make some progress with this stuff!

@Snowdrama
Copy link

Yup but they are not exposed to GDScript, just "notifications" which help the nodes communicate in c++ world.

Interesting, I'm surprised it doesn't expose a signal for it like on_visibility_changed an on_process_mode_changed signal I could see being useful, but I see now.

Neither, the way I implemented this is that active state is simply another property like visibility that can be toggled on or off, without either of them throwing an error. In the code that checks whether a node should render, it simply checks both visibility and node_active, hence it acting like an override.

Ohh that's interesting that makes sense to me, as long as having a node be in one state, setting it inactive, and then setting it active again keeps it the same way it was before setting it inactive I'm happy with that.

Just tested this to be sure; so in this example my_node.visible is true because the "visibility field" is true. If you want to check whether or not the node is "currently visible in the scene" you could call my_node.is_visible_in_tree() which accounts for node_active and parent nodes that have visibility/active state set to false.

Perfect! I was literally just thinking maybe something like an is_visible() would make sense, sounds like this covers that!

Similarly, as I mentioned above I've got is_node_active() to tell if it and all parents are active, and is_node_active_self() to tell if it is active individually

That sounds good to me! makes sense similarly my_node.node_active would be true, even if a parent was inactive, and is_node_active() is the same as is_visible_in_tree() but for node_active might be worth considering naming it is_node_active_in_tree() for consistency with the same function for visibility?

Strictly speaking I don't think it's necessary (so long as something runs the GDVIRTUAL_CALL() macro) but definitely good to have from what I can tell!

Yeah at least on the C++ side having those will probably enable someone to do something cool with them eventually right? haha

@Kalto-Mate
Copy link

If enable and disable are already taken naming-wise, would "awake" and "asleep" be a good compromise? I for one also expect a disabled node to still be reachable and "be there somewhere", just not using all that many resources. The sleeping analogy seems to work better in my head compared to "disable" anyways. For some reason I assume the later means the node is GONE gone.

@AThousandShips
Copy link
Member

"sleeping" is already taken for physics bodies so it's not appropriate to use here

@Kalto-Mate
Copy link

D A R N I...
hmmm... Unconscious? Hibernating? Frozen? (which should absolutely be the physics one)

@AThousandShips
Copy link
Member

Frozen is a different physics thing, but the node isn't really sleeping so I don't think any synonym for it is useful

@AThousandShips
Copy link
Member

But it'd say the naming discussion and confusion here highlights an important thing:
This feature risks being confusing based on what it does or is expecting to do, and that the name is critical to not mislead people as to what it does

@RobProductions
Copy link

But it'd say the naming discussion and confusion here highlights an important thing: This feature risks being confusing based on what it does or is expecting to do, and that the name is critical to not mislead people as to what it does

In that case I think "node active" is probably the clearest name I can put to it as an inactive node will not process and will not render, whereas an active node can do either/both of those things depending on how you set visibility/process_mode. So unless the existing is_enabled() function name which is exposed to users is renamed to process_mode_enabled() (and similar renames for the internal notifications), I think this is the best option if that works for now :)

To that end, I've reworked some names for clarity and fixed up a few things that we discussed above! node_active is now a bool instead of an enum:

image

There are now two active-related signals which are implemented in Node instead of Node3D. node_active_changed is run when the node's active bool changes and node_active_in_tree_changed is run when it or a parent node has changed in such a way that it's become active or inactive in the tree:

image

The node active getters are now named is_node_active_self() and is_node_active_in_tree() to match the visibility functions, and are exposed to the user and work in runtime:

image
image

I've now confirmed the set_node_active call works in runtime from GDScript:

# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta: float) -> void:
	timer += delta;
	child.set_node_active(timer > 1 && timer < 2);
Capture2024-06-24.15-13-30.mov

The new lifecycle functions _on_node_active() and _on_node_inactive() are now working in GDScript when a node's state changes:

func _on_node_active() -> void:
	print("I'm active!");
	
func _on_node_inactive() -> void:
	print("I'm inactive!");

Note that are called each time the "active in tree" status changes, just not just the first time. Behind the scenes they are called just before the node_active_in_tree_changed signal is sent. The last thing to do for these is to run them when a scene first loads up, I'm thinking just after _ready() so that users can perform critical setup steps before doing active-dependent start behavior :)

Lastly, I've implemented active state visibility changes for Node2D as well since it is handled independently of 3D nodes and it was much easier this time thanks to the signals being moved into Node:

image

Left to do: lifecycle calls on scene load, text fade feedback in the scene hierarchy, editor setting to disable checkmarks, testing/documentation. Signals currently still work like before (i.e. they always run regardless of active/process state on sender and receiver), and I might say that if we want signals to not be sent to inactive nodes it could be left for a future PR. There I would recommend we use two ways to call signals either via parameter or with 2 function calls, one that ignores inactive state and one that doesn't. That way we can keep the existing behavior and open up this ease of use feature for more complex stuff. Hope it's looking good for everyone that contributed ideas!

@Snowdrama
Copy link

This feature risks being confusing based on what it does or is expecting to do, and that the name is critical to not mislead people as to what it does.
Understandable, though I'd personally argue that for a beginner with no engine knowledge, the "Active" toggle is actually MORE intuitive as a starting point. It's just a big checkbox that "makes it not do anything" no visibility, no physics, no processing. And then as they become advanced users, they can dig into the minutia of node active vs visibility vs process disabled

Similarly anyone coming from Unity to Godot will already be very familiar with the idea of the "active toggle" as it more closely mimics how Unity's gameObject.SetActive(false) works(funny enough I got to this proposal by searching for a way to "fully" disable a node not just set visible false and process disabled since physics bodies are still active if you do that)

Most gripes I imagine will likely be with people who are already advanced Godot users who are familiar with Visibility and Process, and run into edge cases where they use if node.visible: instead of if node.is_visible_in_tree(): and it returns true even though it's "not actually there" because it's not active. But there are similar edge cases with how visibility works already anyway.

Note that are called each time the "active in tree" status changes, just not just the first time. Behind the scenes they are called just before the node_active_in_tree_changed signal is sent. The last thing to do for these is to run them when a scene first loads up, I'm thinking just after _ready() so that users can perform critical setup steps before doing active-dependent start behavior

That's basically exactly what I expected, they'd be called every time the node becomes active/inactive and are good places to do initialization for tools and pooled objects and stuff!

@RobProductions just to ask because I'm not sure if I've seen it mentioned, but for nodes with collision like StaticBody3D does this remove the body from the physics simulation? For example, if I have a StaticBody3D platform and a Rigidbody3D object on top, and I set it inactive, will the body on top of it fall? Just curious if that was something this covered or has been tested at all. I assume if not, I could probably use the _on_node_active and _on_node_inactive calls to disable it myself but I figured I'd see if it was already covered and I just missed that part.

@RobProductions
Copy link

I got to this proposal by searching for a way to "fully" disable a node not just set visible false and process disabled since physics bodies are still active if you do that

Actually from what I can tell setting process to disabled in 4.3 does stop collision and forces, see below

or nodes with collision like StaticBody3D does this remove the body from the physics simulation? For example, if I have a StaticBody3D platform and a Rigidbody3D object on top, and I set it inactive, will the body on top of it fall?

Let's try it! First I run Rigidbody over Static Body with default state, then I set process mode on the static body to disabled, then I set process mode back to default and disable node_active for the static body:

Capture2024-06-24.17-42-38.mov

So it would appear that process mode disabled does in fact disable collisions and because the active state overrides the can_process and is_enabled check, the same applies here. As you can see in the video I posted earlier, I've already confirmed it stops forces in Rigidbody as well. Hope that's helpful :)

@Kalto-Mate
Copy link

At the end of the day, this addition will work like a shortcut to do things quicker, a broader stroke, because I'm positive some Unity refugees currently go "Oh well, it's not a single call anymore" and have routinely made their own reusable code snippets that mess with several node properties in bulk. This would just make things official, so I can't wait for it to be accepted

@RobProductions
Copy link

RobProductions commented Jun 27, 2024

I noticed that since cameras aren't affected by process_mode or visibility, they needed extra logic to disable when inactive, so I'm currently working on that. However, it's made me think about the lifecycle calls because this is one case where utilizing them internally will make things a lot easier. Here's a quick question I wanted to ask about that:

Currently, after a node has run _ready() it will check whether it's active/inactive and send a notification to run _on_node_active() or _on_node_inactive() for GDScript. Should it instead only run an _on_node_active() on startup if it's active and not do anything if its inactive? I think this is what you guys meant above but I just wanted to make sure! i.e. remove the "else" here:

image

I'm also going to assume that we want _on_node_inactive() to run before the node exits the tree if it was active before that point, right? This would also match Unity's OnDisabled call which is what I think we were going for. If we go this route it should make the camera code a little cleaner, and then after that I pretty much only have documentation/testing left to do! Once I have that started I will make one last update here with screenshots before the PR :) Thanks!

@Snowdrama
Copy link

Snowdrama commented Jun 27, 2024

Should it instead only run an _on_node_active() on startup if it's active and not do anything if its inactive? I think this is what you guys meant above but I just wanted to make sure! i.e. remove the "else" here:
...
I'm also going to assume that we want _on_node_inactive() to run before the node exits the tree if it was active before that point, right?

Exactly you got it. I consider these functions like secondary constructors/deconstructors that are triggered when active/inactive state changes.

So from my perspective yes only _on_node_active() would be called after _ready() if active is true when loading the node.

Then similarly to how _on_node_active is called during the process of setting up the node, _on_node_inactive is called during the process of the node's removal.

So node initialization would be:

_enter_tree
_ready
if node_active == true : _on_node_active

Teardown would be:

_exit_tree
if node_active == true : _on_node_inactive()

though I could also consider flipping these so inactive is before _exit_tree I could see both, but I think having it triggered by the exiting of the tree is probably easier to implement.

That covers cases where if we set it inactive during runtime then it doesn't call _on_node_inactive() when freeing the node when exiting the tree, since it's already inactive.

@OhiraKyou
Copy link

OhiraKyou commented Jun 28, 2024

In theory, the inactive function should be called before exit tree. On disable/inactive would often be used for cleanup of things set up during on enable/active. If possible, that cleanup should also work the same way even when exiting the tree (i.e., while the node is still in the tree).

Also, as a rule, I believe that lifecycle function pairs should be nested so that their setup/cleanup scopes are preserved.

Rough XML visualization of what I mean:

<tree-scope>
	<!-- On enter tree -->
	<active-scope>
		<!-- On active -->
		  <game-loop></game-loop>
		<!-- On inactive -->
	</active-scope>
	<!-- On exit tree -->
</tree-scope>

@Snowdrama
Copy link

Also, as a rule, I believe that lifecycle function pairs should be nested so that their setup/cleanup scopes are preserved.

Yeah which is why I suggested that it possibly could be switched to before _exit_tree() however for me that's not a blocker if it made the implementation significantly more difficult, I'd rather get it implemented and fight over getting that changed later than prevent it from being implemented. Given that the _node_active() is called as a response to _ready() so if it needs to be as a response to _exit_tree() I don't think it it should be a blocker.

@RobProductions
Copy link

however for me that's not a blocker if it made the implementation significantly more difficult

No worries, it was easily added before exit_tree is called as part of the exit tree propagation step, so it will occur in the order you guys suggested :) I've also updated it to be _node_active() instead of _on_node_active() (and similarly for inactive) in order to match consistency of _ready() and _exit_tree() etc. which have no "on" prefix.

Currently facing a bug that when I switch scene tabs in the editor, the new node_active property is reset to false for the root node of the scene you switch to. I'm at a bit of a loss as to why that's happening, best idea is that I'm missing some serialization step or something in regards to root node properties during scene switching, but I don't see any special treatment on any of the other properties for this case so I'm not quite sure. Appreciate any ideas there if anyone knows why that may happen!

Besides that I still have to generate the XML docs and test thoroughly, perhaps with new unit tests as well. Everything else is pretty much finished and cleaned up (should be done within a week or so if I'm lucky with fixing that bug), and it's gearing up to be a hefty PR so I imagine it will take some months to get it reviewed/merged. Will do the best I can though and appreciate your support when it's ready since community interest helps with getting it seen faster :)

@Snowdrama
Copy link

Currently facing a bug that when I switch scene tabs in the editor, the new node_active property is reset to false for the root node of the scene you switch to. I'm at a bit of a loss as to why that's happening, best idea is that I'm missing some serialization step or something in regards to root node properties during scene switching, but I don't see any special treatment on any of the other properties for this case so I'm not quite sure. Appreciate any ideas there if anyone knows why that may happen!

That is interesting, so it doesn't happen for visibility, I'd have to actually look into it but that's interesting, I wonder what happens when you switch tabs. Does it keep the whole scene loaded? Or does it like cache it to disk or something temporarily. I haven't looked too much at the engine code itself but I can check it out. Is there a fork/branch I could see the current changes you've made for this? Not that I think I'll have any luck haha

@RobProductions
Copy link

RobProductions commented Jul 8, 2024

@Snowdrama appreciate the ideas, I think I figured it out! What happens when you switch tabs is that the scene is removed from the special "editing nodes" list and the new contents are loaded into the tree, so since the new lifecycle call _node_inactive() was setting nodes inactive on exiting tree, it was happening during tab switch. I've fixed it now but noticed a new problem in that the children of the root aren't receiving the notification when the application ends, however I'm pretty confident I can track this one down easily.

Feel free to check out my progress so far here, though it still has the issue I mentioned above and no tests/documentation yet. I was busy with another PR recently but I'm back on it this week to put the finishing touches on this!

@Snowdrama
Copy link

Snowdrama commented Jul 8, 2024

Ohh interesting, I was thinking about this and how this may be a desirable thing? I was curious of how @tool affects this, as I know some people were planning to use the active/inactive lifecycle calls for tools in the editor.

Does _node_active() get called when opening a tscn into a tab?
Does closing the tab/scene call _node_inactive()?
Is it desirable that _node_active() and _node_inactive() get called when switching tabs?

At the very least it probably shouldn't do these things if it's not marked as @tool but if it is a tool, then might be worth discussing, as it would reflect a similar expectation to how _process and _ready also gets called in the editor if it's marked as a @tool

Consider this script, if you close or open the tab _exit_tree _enter_tree and _ready are all called

@tool
extends Node

func _ready():
	print("_ready")

func _enter_tree():
	print("_enter_tree")

func _exit_tree():
	print("_exit_tree")
	
#_process and _physics_process are also called, but I omitted them just for clarity

Note that if marked as a @tool the _exit_tree and _enter_tree methods ARE called but interestingly _ready is only called on scene load, which I think makes sense.

So I'd imagine that the expectation would be that _node_active() and _node_inactive() would also be called? At least if it's marked as a @tool But maybe not, maybe once it's active, it doesn't become inactive unless it's toggled inactive OR the scene exits, similar to how _ready is only called on load, but not on tab switch.

Godot_v4.3-beta2_mono_win64_AvoBGsZmPg.mp4

@RobProductions
Copy link

I was curious of how @tool affects this, as I know some people were planning to use the active/inactive lifecycle calls for tools in the editor.

It should and does currently, same as the other lifecycle calls because it uses the same system to run them. Your demo actually helped a lot with getting me to think about when _node_active() should be called, and I've realized instead of being tied to _ready it should simply happen after ready but still on each time _enter_tree happens. I've adjusted it and it makes a lot more sense this way I think!

Does _node_active() get called when opening a tscn into a tab? Does closing the tab/scene call _node_inactive()? Is it desirable that _node_active() and _node_inactive() get called when switching tabs?

Yes, yes, and yes imo. Here we have the same code you applied with the new lifecycle calls:

func _ready():
	print("_ready")

func _enter_tree():
	print("_enter_tree")

func _exit_tree():
	print("_exit_tree")

func _node_active():
	print("_node_active");
	
func _node_inactive():
	print("_node_inactive");

And the script is marked as @tool. Then, this is what we get in the console with the updated method I mentioned above:

Capture2024-07-08.13-30-12.mov

As you can see, _ready() is only called once on scene load (i.e. in the editor's case, when it loads) but all subsequent tab switching and enter/exit calls will run the lifecycle functions in the desired order :) As for this:

noticed a new problem in that the children of the root aren't receiving the notification when the application ends

It turns out that exit_tree is not actually called on nodes when you manually close out of the game window, so there's no way for _node_inactive to be run while it's hooked to that. I think this is actually fine since it is consistent with the tree lifecycle calls and it would be much more convoluted & make less sense to try and also link it to application close. So I will leave it as is for the PR! Now looking into creating test cases for the new property so we'll see how that goes lol

@RobProductions
Copy link

Finally submitted the PR which will close this proposal :) Feel free to check it out, test it, or show your support here: godotengine/godot#94144

Since it's a big change and especially since we are in 4.3 feature freeze right now, it will probably take a while to iron out issues and get it merged but I will keep an eye on it and make any changes necessary. Let me know how it works for you if you'd like to play around with it! Thanks again to everyone that helped with ideas/feedback!

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.