Add colored margin in Inspector for arrays and dictionaries #75482
Conversation
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
d432cfb to
8188ea4
Compare
fire
changed the title
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
8188ea4 to
ffea158
Compare
YuriSizov
changed the title
|
Implementation seems pretty straightforward, but some adjustments are required. I'll give it a functional test afterwards, but it's probably good for inclusion into 4.1. |
|
Thanks for the contribution! I am not 100% against the idea, but I kind of find useful that only sub-resources are highlighted. It is kind of making clear which parts of the properties might be things that are not part of the current scene. Thus adding highlighting to dictionaries is a bit confusing to me, and adds a lot of noise. I'd think I'd rather have bit more feedback on that solution before merging it, and that it might require a dedicated proposal to be sure it has enough baking. But that being said, I don't have a better solution for now to solve the readability issue, so it might be an acceptable solution for now. |
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
2 times, most recently
from
6c8b8e0 to
8b03965
Compare
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
2 times, most recently
from
0ecc9ac to
557bb1f
Compare
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
557bb1f to
2ef1863
Compare
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
2 times, most recently
from
7d4763e to
d395b38
Compare
|
You could format those previews in your reply using tables https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/organizing-information-with-tables |
There was a problem hiding this comment.
Not sure about the name. This is (almost) the only thing in the entire engine called "coloration". I think the setting could actually be more obvious and be called nested_color_mode.
The extra delimitate_all_container_and_resources is definitely iffy, especially since it does nothing for one of the above setting's values. Not to mention the huge name.
My opinion is that, as is, there's little reason to want this disabled as it causes this void where some delimiters should be. Not exactly pleasing.
If that is addressed, it could be part of the original setting, maybe?
doc/classes/EditorSettings.xml
Outdated
| @@ -659,6 +659,8 @@ | |||
| <member name="interface/inspector/auto_unfold_foreign_scenes" type="bool" setter="" getter=""> | |||
| If [code]true[/code], automatically expands property groups in the Inspector dock when opening a scene that hasn't been opened previously. If [code]false[/code], all groups remain collapsed by default. | |||
| </member> | |||
| <member name="interface/inspector/coloration_mode" type="int" setter="" getter=""> | |||
There was a problem hiding this comment.
If you could roughly fill in this it would be nice (I could help later if the feature is liked). As I like to say, no one knows the thing best than the one that created it in the first place.
|
I like the For the other parameter I'm in favor of making it go away. I kept it as it acts as it used to before this PR this is like a legacy mode (ie. the space was already present before this PR). I could make it true except for the all resources and rename this mode Legacy? |
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
2 times, most recently
from
c55ac5a to
4ad9c48
Compare
|
I finally came back to this hopefully for the last time I reduced the size when there is no border (see comparison below). I also changed the name of the setting and added description for both settings. I'm not sure of how to integrate inside the other setting. |
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
4ad9c48 to
91af2c2
Compare
|
Changing color mode to Resources has no effect on built-in resources. |
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
e89e41d to
b2feff5
Compare
I inverted Resource and External Resource in the list because realized it made more sense in this order thought I had done the same inversion in the enum but in fact did not :( Fixed it. |
|
External Resources coloring does not work if the external resource is inside internal resource (e.g. AtlasTexture with texture). Nor sure if intended 🤔 |
This is not the expected behavior. I'll try to investigate |
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
b2feff5 to
f866e88
Compare
First resource is internal two following are external. |
|
The last update added a merge commit, could you rebase on |
Apply suggestions from code review Co-Authored-By: A Thousand Ships <[email protected]> Co-Authored-By: Tomek <[email protected]>
ajreckof
force-pushed
the
color-hint-for-arrays-and-dictionnaries
branch
from
f866e88 to
cba9606
Compare
|
Thanks! |
Closes godotengine/godot-proposals#2018.
Closes godotengine/godot-proposals#6987.
This PR aims to enhance readability of nested data by adding a colored border for arrays and dictionaries as it was done for Resources in #45907.
I've updated a bit this PR to have better readability for dictionaries especially or the Panel around the element to add. Here is how it nows looks like with multiple nested data (Resources, Arrays and Dictionaries) :
