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

Use different "reset" icons for instanced scene properties in the inspector #2581

Open
Calinou opened this issue Apr 9, 2021 · 5 comments
Open

Comments

@Calinou
Copy link
Member

Calinou commented Apr 9, 2021

Describe the project you are working on

The Godot editor 🙂

Describe the problem or limitation you are having in your project

Right now, the Reset button next to a property looks identical in the inspector, regardless of its "reason":

  • A property can have a reset button because its value differs from the node's default value.
  • For instanced/inherited scenes, a property can have a reset button because its value differs from the parent scene's default value.

This can be confusing to people who make heavy use of instanced scenes.

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

Use slightly different icons (and/or different colors) to distinguish between a reset icon that is shown because the property is different from its default value, and a reset icon that is shown because the instanced scene has a value different from its parent.

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

The Reset icon could have a different color when comparing against a parent scene (compared to the default value).

For instanced scene properties, we could also have an icon that means "modified from the default value, but identical to parent scene". This icon could take the form of an asterisk (similar to the "modified" marker used in the window title). This icon wouldn't be clickable, though.

See godotengine/godot#45536 (comment).

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

No, as EditorInspector is drawn by the engine's C++ code.

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

See above.

@dalexeev
Copy link
Member

I was just thinking about another issue, but it also related to the reset button. It seems to me that we should distinguish between default values ​​and values ​​that are the same as default values. This is done for custom theme properties (which have a checkbox instead of a reset button in the inspector), but for other properties it is not possible to save a value to the scene file if the value matches the default. See comment on my PR godotengine/godot#39072. The proposed solution, using black as a special value (at which the behavior of the property suddenly starts to differ) does not look entirely clean IMO.

With regard to this proposal, it seems confusing to me. As many as 3 options for the type of reset button and the 4th state is its absence. The current behavior IMO is not a bug, but the correct behavior. This is how inheritance works, the change must be calculated relative to the default of the last inherited class/scene, not its ancestors.

@dalexeev
Copy link
Member

For example:

Control

FocusMode focus_mode 0

BaseButton

FocusMode focus_mode 2 (parent override)

If it works like this for classes, then it should work in the same way for scenes. No need to overcomplicate.

@timothyqiu
Copy link
Member

I think a special reset icon is unnecessary because the action is still "reset the value to be the same as its parent". And once clicked, the asterisk icon is revealed anyway, indicating the default value is changed somewhere in the inheritance chain.

@Calinou
Copy link
Member Author

Calinou commented Jul 26, 2022

I think a special reset icon is unnecessary because the action is still "reset the value to be the same as its parent". And once clicked, the asterisk icon is revealed anyway, indicating the default value is changed somewhere in the inheritance chain.

Should I amend this proposal to keep only the asterisk icon?

@timothyqiu
Copy link
Member

Should I amend this proposal to keep only the asterisk icon?

I think so since comments in the corresponding PR also have similar thoughts.

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.

3 participants