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

Make it possible to explicitly assign the currently default value to an exported property #9482

Open
MajorMcDoom opened this issue Apr 8, 2024 · 8 comments

Comments

@MajorMcDoom
Copy link

MajorMcDoom commented Apr 8, 2024

Describe the project you are working on

Any Godot project.

Describe the problem or limitation you are having in your project

Currently, an exported property only exists in the resource as an override if it is not equal to the default value of the property. This means it is impossible to explicitly state that a property should be of a value that just happens to be the current default. If the default value changes, the override entry gets wiped out.

Example scenario: I have 15 right-handed characters, and 15 left-handed characters. However, only half of them have saved handedness properties, while the other half just omit it because they are on the default value. If I change the default value of the handedness property, 15 of my characters would all of a sudden change handedness, and I would now have to re-assign handedness to all of them.

This not only applies when changing the default value from code, but also when changing the base scene if your scene is inherited or instantiated.

In short: there is always one value for any property for which the user has no way of explicitly and reliably saying "this is what I want this property to be."

EDIT: There does exist a "force override property" feature for inherited/instantiated scenes, but it has the following problems:

  • It only works for inherited and instantiated scenes, not for base scenes or resources or settings or anything else that has the override UX.
  • It isn't discoverable because you have to right click on a property to find it.
  • It adds an unnecessary layer of complexity with a new concept and interaction.
  • It further highlights the fact that overriding is unreliable because you have to take additional measures to force it to do what you originally intended.

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

Make the saving of an exported property not based on its value, but based on intent-to-override.

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

Right now, the inspector UX shows a little clear override icon ↪️ if a property is different from its source. Clicking it reverts the value to the source value, and as a result, the override is cleared from the resource. Changing the value itself to the default value also erases the override and hides the icon.

With the proposed changes, the UX is now this:

  • The clear override icon ↪️ simply reflects if there is an override entry saved in the resource, period.
  • Clicking the icon erases the override, and hides the icon. This is an explicit clear operation, and is the only way to clear an override.
  • Setting the value at all adds an override to the resource, and causes the icon to appear. This is true even if the value is changed to the same value.

This is the same UX used by Unity and Unreal. This separately addresses the distinct intents of "I want this value" vs. "Leave it to the source". The caveat of course is that if you set a value to the same as the source, you might accidentally commit an override when you really intended to just leave it un-overridden. However, the UX mitigates this because the presence of the ↪️ icon is an immediate, obvious, and reliable reflection of whether or not there is an override. If you don't want the override, just get rid of it.

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

No.

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

This is a core editor feature.

@timothyqiu
Copy link
Member

You can use Pin Value on the property, so that its value is saved even when matching the default.

image

@MajorMcDoom
Copy link
Author

MajorMcDoom commented Apr 9, 2024

You can use Pin Value on the property, so that its value is saved even when matching the default.

![image](https://private-user-images.githubusercontent.com/372476/320743540-908d0069-324a-4d21-8e66-74bf32afda38.png

The "force override" has a few problems:

  • It only works on inherited scenes, not on normal scenes, resources, or settings.
  • It introduces an unnecessary layer of complexity that is difficult to justify. It further highlights the fact that overriding a value, an operation which should yield a reliable result, is so unreliable in Godot that you have to say "just in case, please don't change what I have done."
  • It is not very discoverable.

The present proposal would cover all user intents with a single "override" concept and a single UX interaction without requiring the introduction of "force override" and "pinning". Plus it would be consistent with other engines. This is one case where I feel like Godot shouldn't unnecessarily deviate from standards on already-solved UX problems, for no clear added benefit.

@djrain
Copy link

djrain commented May 28, 2024

There should also be some annotation when defining the property to override by default. Or actually, I'd prefer "always override", so there's no way to mess it up accidentally. Or both options.

@MajorMcDoom
Copy link
Author

There should also be some annotation when defining the property to override by default. Or actually, I'd prefer "always override", so there's no way to mess it up accidentally. Or both options.

Could you describe what "mess it up accidentally" would look like in the context of the proposed feature?

@djrain
Copy link

djrain commented May 28, 2024

Could you describe what "mess it up accidentally" would look like in the context of the proposed feature?

I guess clicking the clear override icon. A level designer could easily hit that not realizing it changed something important.

I can't think of any scenario I'd want to override by default but then not override it.

@MajorMcDoom
Copy link
Author

Could you describe what "mess it up accidentally" would look like in the context of the proposed feature?

I guess clicking the clear override icon. A level designer could easily hit that not realizing it changed something important.

I can't think of any scenario I'd want to override by default but then not override it.

I think you might be misunderstanding something. In the context of this proposal, the "clear override" icon is just the current "revert" icon, and it does the exact same thing as it does now - it reverts the value back to the default, or the inherited value. It's an intentional action, and it's no more error-prone than it currently is.

@djrain
Copy link

djrain commented May 28, 2024

@MajorMcDoom I understand. My point is, that if I know I want some property always stored, then ideally the UI should not provide any way to delete the override. How exactly this would work alongside your proposal though, I'm not sure...

@MajorMcDoom
Copy link
Author

MajorMcDoom commented May 28, 2024

@MajorMcDoom I understand. I just wouldn't want the UX to provide any way to delete the override if I don't need that option for some particular property. safer that way :)

Fair enough, though that's not specifically an "override" thing - the possible human error you described is the current reality for any value in the inspector, because one can always hit the revert button by accident, just as they can drag a number field, or clear a checkbox by accident. I don't think the revert button is a case deserving of special protection measures.

In any case though, I think that should be an additional proposal if this one goes through, since the present proposal is about removing the automatic clearing of override state when changing to a specific value. It would already reduce errors greatly because it gets rid of one way to clear an override.

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