Add visibility_condition to allow Editor Only nodes.

[EMBYRDEV]

Implements godotengine/godot-proposals#3433.

Sorry for closing the last one, I made a bit of a mess of history.

Adds visibility_condition to Node3D and CanvasItems. Super useful for level design markup.

Also adds debug option to view Editor Only nodes in the running project.

Production edit: Closes godotengine/godot-proposals#3433

[dalexeev]

See the comments about the property name in the previous PR.

@EMBYRDEV For the future, there is no need to open a new PR if you "broke" the previous one, you can just force-push the branch. Recreating the PR makes the discussion more difficult.

[EMBYRDEV]

See the comments about the property name in the previous PR.

@EMBYRDEV For the future, there is no need to open a new PR if you "broke" the previous one, you can just force-push the branch. Recreating the PR makes the discussion more difficult.

Ahh yeah thank you! I actually tried to rebase the branch but it's been a while since I've PRd and I made the mistake of using GitHub Desktop to make the final push instead of manually force pushing which just made the situation worse.

[EMBYRDEV]

One note that is useful to bring forward to this new PR:

If a node is marked visible but is hidden in editor then the visibility icon will show as an open eye but greyed out to inform to inform the user that something is overriding it.

[AThousandShips]

I think the visualisation needs a look by someone on the accessibility area, I don't think this is very clear even to me and I don't have any vision impairment (also needs to be checked if running on dim or eye strain prevention modes etc.), I'd suggest that a half open eye would be better, or an eye that is cut in half

[EMBYRDEV]

I think the visualisation needs a look by someone on the accessibility area, I don't think this is very clear even to me and I don't have any vision impairment (also needs to be checked if running on dim or eye strain prevention modes etc.)

Yeah, I agree it could stand out a little more. Currently it's inheriting the colors that the closed eye icon gets when visibility is disabled. However I think that having a new icon made could improve readability, perhaps an open eye with an asterix.

That being said it's also only really used in the case where the visibility mode is set to Runner Only and a developer may be confused as to why they can't see the Node in editor.

[AThousandShips]

Note that the whole purpose of icons in UI is to provide information at a glance, the dim version also has the drawback of possibly being hard to identify if you only have others of the same state nearby, as it is contrast dependent

[Calinou]

We have a Xray eye icon we use for gizmos that we could use in this particular scenario (check editor/icons). Alternatively, you can display the icon with a different color.

[YuriSizov]

Adds visibility_condition to Node3D and CanvasItems. Super useful for level design markup.

Should this also be added to Window?

[EMBYRDEV]

Should this also be added to Window?

I can't think of too many reasons where that would be useful but I think it's probably worth me adding for the sake of consistancy. I was a bit surprised that visibility wasn't handled on the Node level, even if some subclasses wouldnt make use of it.

[YuriSizov]

I was a bit surprised that visibility wasn't handled on the Node level

I haven't checked this PR in detail, but the idea of Editor Only nodes isn't just about visibility, is it? It's enabling or disabling them completely. And for that it should be handled on the Node level (and in PackedScene).

[EMBYRDEV]

I haven't checked this PR in detail, but the idea of Editor Only nodes isn't just about visibility, is it? It's enabling or disabling them completely. And for that it should be handled on the Node level (and in PackedScene).

Nope this is purely for visibility as outlined in the feature proposal.

Editor only nodes in general could be useful but I think it's a bit of a rabbit hole. My main engine is Unreal and they handle things with visibility. I agree with @Calinou proposal that any sort of logic changes like removing nodes should be handled by a script checking the editor hint.

This change is mainly to enable editor only level markup visualisation for things like triggers, AI points of interest and logic entities. For example this type of Node (in my Unreal game) would be built by setting the cube and text nodes to only visible in editor. We want to keep those nodes around for if the player wants to enable a debug mode at runtime that shows them.

[EMBYRDEV]

Personally I do feel that the visibility route makes a lot more sense than outright removing nodes.

I think that it would be useful to add a feature in the future that can remove nodes based on a tag system in the future. For example I have a game where I'm working on a new quest but I dont want the quest items to ship with the game until I enable that quest's tag. Essentially some form of stripping out content from the scene that isn't in the "enabled features" list. Every AAA game I've worked on has had some sort of system like that.

[EMBYRDEV]

There is a seperate PR for editor-only nodes that takes the other approach.

#56446

[YuriSizov]

Nope this is purely for visibility as outlined in the feature proposal.

Right, that particular proposal is indeed only about visual changes. I think that to a lot of users it would be unexpected that a node marked as Editor Only would still run code in the project. This also adds complexity to visibility resolution and to updating it at runtime.

That is to say, that for your example it makes as much sense to completely remove those nodes, or rather never to add them in the first place. So I like the approach from #56446

[EMBYRDEV]

That is to say, that for your example it makes as much sense to completely remove those nodes, or rather never to add them in the first place.

I disagree personally, as it may introduce some complexity elsewhere. For example, the editor only text for the counter nodes in my game updates to show it's current state for debugging reasons. That way a player can show editor nodes and use that to debug the current game state. If the node gets removed then I will need to check that the node exists every time I update the state.

That being said #56446 is a valid approach and would be good to get the ball rolling on at least one of these options.

Another option would be to add a new node type that toggles it's own visibility based upon editor hint and visualisation options. That way all the editor only visible nodes can be children of it. Would make sense ot have this in native code rather than a script because of the hundreds/thousands of markup nodes that can be present in large levles.

[YuriSizov]

If the node gets removed then I will need to check that the node exists every time I update the state.

That makes sense to me, it makes you very aware of editor only nodes and how they behave. It also works in the most predictable way, because it's explicit.

Another option would be to add a new node type that toggles it's own visibility based upon editor hint and visualisation options. That way all the editor only visible nodes can be children of it. Would make sense ot have this in native code rather than a script because of the hundreds/thousands of markup nodes that can be present in large levles.

I don't think it would work as a node (or rather several nodes) in the engine where this is their sole functionality. For example, it would be hard to use with controls, since their tree structure is important and can't have arbitrary nodes in the middle. But that can be implemented with a trivial custom node per project. That said, I agree that we should solve the problem in the engine somehow. I would very much want to use debug nodes like that in my projects too.

[EMBYRDEV]

Out of curiosity, can anyone think of any nodes types that would be useful to mark as Editor Only that aren't purely visual?

[AThousandShips]

I think that approaching this without a unified approach isn't constructive, there might be reason to have nodes that are only hidden in the exported/running game, and reason for nodes that are completely removed from the exported/running game, but I do think it requires a unified approach to avoid confusion or redundancies

[EMBYRDEV]

I think that approaching this without a unified approach isn't constructive.

I think that iteration is how we make things better. This feature works and provides a lot of value to many users. Adding a new export option "strip nodes with editor-only visibility" would definitely be a good follow up PR when I get time.

However I think allowing both #56446 and this PR just sit in purgatory instead of providing the useful feature to users is the wrong approach and slows development.

[EMBYRDEV]

Definitely happy to update the UI icon if desired but I think that stripping the nodes as an option is outside of the scope of this PR and the time I have available. Would be happy to add this option in the future.

[Flynsarmy]

Out of curiosity, can anyone think of any nodes types that would be useful to mark as Editor Only that aren't purely visual?

There are plenty of reasons you'd want stuff visible in editor but hidden by default in game. Take for example a progression tree where you only want the next tier to be visible in game but while developing you want to see the whole tree. Editor only option here means you don't need a script to hide everything on load when the game starts up.

[EMBYRDEV]

There are plenty of reasons you'd want stuff visible in editor but hidden by default in game. Take for example a progression tree where you only want the next tier to be visible in game but while developing you want to see the whole tree. Editor only option here means you don't need a script to hide everything on load when the game starts up.

Yeah that's kinda what I was getting at. This is just hiding nodes, not removing them from the tree which is what they were advocating for. I feel like 90% of cases these nodes are visual only so hiding them in game with a debug option to show them again is the ideal setup.

[EMBYRDEV]

Closing due to lack of interest.

[Flynsarmy]

I'm interested 😞

IMO 'visible' should be renamed 'visible in game' and a second checkbox 'visible in editor' added (both checked by default).

[EMBYRDEV]

@YuriSizov just giving this a once over, do you think that this PR is likely to be merged if some small changes are made or does the project leadership wish to go with a different approach.

I don't really wanna leave this open and in purgatory if this isn't the path you guys wanna take.

[YuriSizov]

@EMBYRDEV There hasn't been any discussion about this feature since we last talked in the comments above. No extra consensus by any party has been reached, no other opinions shared.

The project leadership doesn't make such decisions on their own, it's not what their function is. There needs to be an agreement between interested parties. If there are only a handful of people and there is no agreement, then it's likely better to preserve the status quo than to make the change. That said, there is an agreement that there is a problem, between us here at least. We just don't agree what kind of solution is required.

PS. I'm not a part of the leadership, nor represent it in any capacity. I'm talking on my own behalf about what I think is going to be a problem or an expected behavior by users. As such, my opinion carries as much weight as yours, or anyone else's. For what it's worth, #56446 already has plenty of support. So if we're comparing which one to pick, that other PR looks more promising. There were some concerns raised a year ago and yet not addressed, but I think it's possible to see it across the finish line. You can coordinate with Geometror and help, if you want.

[Flynsarmy]

^ This is great communication. Thanks Yuri!

[EMBYRDEV]

Thanks for the response Yuri! And all that makes total sense. If we're more likely to go in the direction of #56446 then I didn't wanna muddy the waters by leaving this open :)