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 an editor_only property to Node (and remove it from specific uses such as Light3D) #2512

Closed
BastiaanOlij opened this issue Mar 27, 2021 · 5 comments

Comments

@BastiaanOlij
Copy link

Describe the project you are working on

Dungeon crawler

Describe the problem or limitation you are having in your project

I very often add objects as visual guides to scenes that are meant to be extended.
For instance in this screenshot I've added Left/Right/Top/Bottom labels to easily recognize the orientation of a room I'm working on:
image

Right now these nodes are hidden when the game starts but it's still overhead to create these objects, compile their shaders and load images that are only needed when editing.

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

I suggest adding an "editor only" property to the node section of a node. When ticked this prevents the node and any of its children from being loaded in runtime.

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

See above

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

I'm currently working around it by making these nodes available in _ready but it's wasteful

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

No this is a change that needs to happen in Godots scene loader.

@Calinou
Copy link
Member

Calinou commented Mar 27, 2021

This Editor Only property already exists for certain nodes such as Light3D, but there is no universal toggle that works for all nodes. I discussed this with reduz a while ago and he wasn't really in favor of moving this Editor Only property to Node (I don't remember why).

@Calinou Calinou changed the title Editor only nodes Add an editor_only property to Node (and remove it from specific uses such as Light3D) Mar 27, 2021
@Calinou Calinou added this to the 4.0 milestone Mar 27, 2021
@Xrayez
Copy link
Contributor

Xrayez commented Mar 27, 2021

This Editor Only property already exists for certain nodes such as Light3D

Seems like Light2D, Light3D and ReferenceRect:

PS D:\src\godot\gd4> rg editor_only
scene\gui\reference_rect.cpp
40:             if (Engine::get_singleton()->is_editor_hint() || !editor_only) {
64:void ReferenceRect::set_editor_only(const bool &p_enabled) {
65:     editor_only = p_enabled;
69:bool ReferenceRect::get_editor_only() const {
70:     return editor_only;
80:     ClassDB::bind_method(D_METHOD("get_editor_only"), &ReferenceRect::get_editor_only);
81:     ClassDB::bind_method(D_METHOD("set_editor_only", "enabled"), &ReferenceRect::set_editor_only); 
85:     ADD_PROPERTY(PropertyInfo(Variant::BOOL, "editor_only"), "set_editor_only", "get_editor_only");

scene\gui\reference_rect.h      
41:     bool editor_only = true;
54:     void set_editor_only(const bool &p_enabled);
55:     bool get_editor_only() const;

scene\3d\light_3d.cpp
171:    if (editor_only) {
179:    if (editor_only) {
197:void Light3D::set_editor_only(bool p_editor_only) {
198:    editor_only = p_editor_only;
202:bool Light3D::is_editor_only() const {
203:    return editor_only;
229:    ClassDB::bind_method(D_METHOD("set_editor_only", "editor_only"), &Light3D::set_editor_only);
230:    ClassDB::bind_method(D_METHOD("is_editor_only"), &Light3D::is_editor_only);
280:    ADD_PROPERTY(PropertyInfo(Variant::BOOL, "editor_only"), "set_editor_only", "is_editor_only");

scene\3d\light_3d.h
81:     bool editor_only = false;
102:    void set_editor_only(bool p_editor_only);
103:    bool is_editor_only() const;

scene\2d\light_2d.h
56:     bool editor_only = false;
86:     void set_editor_only(bool p_editor_only);
87:     bool is_editor_only() const;

scene\2d\light_2d.cpp
44:     if (editor_only) {
52:     if (editor_only) {
69:void Light2D::set_editor_only(bool p_editor_only) {
70:     editor_only = p_editor_only;
74:bool Light2D::is_editor_only() const {
75:     return editor_only;
235:    ClassDB::bind_method(D_METHOD("set_editor_only", "editor_only"), &Light2D::set_editor_only);
236:    ClassDB::bind_method(D_METHOD("is_editor_only"), &Light2D::is_editor_only);
281:    ADD_PROPERTY(PropertyInfo(Variant::BOOL, "editor_only"), "set_editor_only", "is_editor_only");

I discussed this with reduz a while ago and he wasn't really in favor of moving this Editor Only property to Node (I don't remember why).

It makes sense to me to have a dedicated property for this especially when we have editor_description property exposed. If bloat is concern, then editor-only properties can be grouped easily in the inspector, but I don't quite see what else could be added.

Introducing editor_only property also means that classes are likely going to be refactored or rewritten when it comes to code wrapped in TOOLS_ENABLED macro, which is also fine (editor-only code would still be compiled out, it just means that we now have an opportunity to control the run-time behavior).

For instance in this screenshot I've added Left/Right/Top/Bottom labels to easily recognize the orientation of a room I'm working on

See also #115 (can be made to work as editor-only as well).

@Calinou
Copy link
Member

Calinou commented Mar 27, 2021

Also, following godotengine/godot#46191, we may want to add an enum to control where the node is shown or instantiated. Therefore, instead of adding an editor_only property, I suggest adding the following:

  • Editor and Runtime (default)
  • Editor Only: The node is shown in the editor and hidden at runtime (or freed to improve runtime performance?).
  • Runtime Only: The node is hidden in the editor and shown at runtime.

@LikeLakers2
Copy link

The node is shown in the editor and hidden at runtime (or freed to improve runtime performance?).

I don't think either option (hiding it or freeing it) is necessarily better than the other. Perhaps you could split this into "Hide at Runtime" and "Delete at Runtime"?

@Calinou
Copy link
Member

Calinou commented Mar 18, 2022

Closing in favor of #3433, which is a more detailed proposal.

@Calinou Calinou closed this as completed Mar 18, 2022
@Calinou Calinou removed this from the 4.0 milestone Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants