Disable editing properties in foreign resources #63282
Conversation
|
Yay, I've had more than one case of editing something and then "wtf where did my changes go?", presumably due to this. |
|
Looks good! Could you increase the opacity of read-only properties slightly? The text contrast rate against the background is a bit low right now. |
|
@Calinou How does this look? I changed the blending to pure black and pure white on the dark and light themes respectively from 0.5 to 0.25 for the readonly color That being said, I notice the the warning color is pretty bad on the light theme. That might be worth chaning too, but I'm not sure what a good colour would be. |
Looks good to me now 🙂 |
SaracenOne
force-pushed
the
disable_foreign_resource_edits
branch
from
19da8be to
61bd0ad
Compare
|
Okay, it's now been updated with improved read-only colors |
|
Awesome, you are my hero! |
|
How does it relate to #59496? Editing foreign resources in the inspector should already be impossible. |
| @@ -212,7 +253,7 @@ void EditorResourcePicker::_update_menu_items() { | |||
| } | |||
|
|
|||
| // Add options to convert existing resource to another type of resource. | |||
| if (edited_resource.is_valid()) { | |||
| if (is_editable() && edited_resource.is_valid()) { | |||
There was a problem hiding this comment.
Keep in mind if the base scene is editable (instantiated or inherited), even if the subresource is not editable you can still replace it by another custom one and this change will be kept.
There was a problem hiding this comment.
As far as I'm able to tell, the behaviour invoked by using the is_editable function should be correct, as in, the conversion options won't appear if a resource is part of another non-editable resource, but it will still appear if its a node property, irrespective of whether its instantiated or inherited.
|
Really good work, some changes noted and needs a rebase. |
|
@KoBeWi I think both should be complementary, possibly with an extra option in the picker "Edit in its own scene" |
|
This feature also should be useful to extend VCS support to show locked files non-editable too. |
|
Okay, I shall update this and address the concerns. I like the notion of extending this to VCS locking; that slipped my mind |
|
Reminder to update this, would be good to have for Beta. |
|
Shoot, sorry about the delay. Other things have been causing a lot of distractions for me lately. |
SaracenOne
force-pushed
the
disable_foreign_resource_edits
branch
3 times, most recently
from
4ce8795 to
1f9ea76
Compare
|
Okay, @reduz, I've made changes to this and I think it's okay now |
|
Is this supposed to handle every property? It's still possible to edit a shader in a foreign shader material. |
|
@KoBeWi This is inspector only, for the other editors there are separate PRs |
SaracenOne
force-pushed
the
disable_foreign_resource_edits
branch
2 times, most recently
from
2db90a2 to
e8215d0
Compare
from imported scenes or objects returning true from a function named '_is_read_only' and disable resaving imported resources.
SaracenOne
force-pushed
the
disable_foreign_resource_edits
branch
from
e8215d0 to
dd814a0
Compare
|
The SVG icon should be properly optimized in the latest version. I followed the instructions @KoBeWi linked, so it should be okay unless I did something wrong. |
|
Thanks! |
This PR adds new behaviour to the inspector to prevent the user from accidentally editing properties of resources embedded in other scenes (foreign resources) since these will not be saved and can lead to changes accidentally being lost the user assumes will be persistent. Editing these resources can still be possible by right-clicking on them and selecting 'edit', which will follow the pre-existing behaviour of opening up the corresponding scene and displaying it in the inspector. For resources which are embedded inside of imported scenes (those derived from imported formats such as GLTF) however, the 'edit' option will be replaced by an inspect option which will still switch the inspector this resource, but will not change the scene and instead display it in a 'read-only' fashion.
The InspectorDock warning button has also been changed to provide clarity when inspecting read-only objects. It is also now considered a more general info button due to considering the displaying of a read-only state to be more informative than warning-worthy, which also includes a new icon when displaying something considered information rather than a warning (flipping an existing exclamation mark vector and putting it inside a circle, a true stroke of genius! Clearly, I must be made Godot's new resident graphics designer)
Furthermore, read-only mode is also applied to remote debug inspector objects with its own contextual information.
Intended to compliment similar changes to bespoke editor plugins in:
#63249 #63245 #63253 #63252 #63251