Change dictionary editor

[ajreckof]

implements godotengine/godot-proposals#5775
This is a huge refactor of the dictionary editor.

Edit : as this was too big of a refactor as a monolithic PR I decided to split as small steps I will keep updated below what has already been done with the linked PR

What has been done :

• Keep the type of the last added item (Dictionary Editor: Keep the type of the last added item. #88636)
• Fix copy paste on dictionary elements (Fix update property for dictionaries so that it updates only what is necessary. #88231)
• Fix sub EditorProperty generation (Fix update property for dictionaries so that it updates only what is necessary. #88231)
• Gives the possibility to edit the key (Add the possibility to edit dict keys. #90258)
• better handle invalid keys ( already present keys and maybe null)
• Rework the right click popup on items to show more appropriate options
• Add better ways to edit keys (double click and/or enter shortcut as node rename)
• Improve readability of non-foldable bottom editors

It is recommended to test this PR with #75482 as it gives better readability. The two PR do not have real conflicts so you should have no problem merging them but for simplicity you can find an already merged version here -> https://github.com/ajreckof/godot/tree/change_dict_editor_+_colored_margin

Enregistrement2.mp4

with #75482 :

without #75482 :

Popup when editing value:

Popup when editing key:

[don-tnowe]

Okay, the idea with swapping the key label with the property editor is pretty clever! You can only edit one key, too, so it's never confusing. There are some bugs:

• Types with a bottom editor (Vec3, Vec4, Plane, Quat...) show at the bottom in both modes;
• Only some types are kept when adding an entry: String, StringName, NodePath, Array, Dictionary, Object revert to null both in value and key;
• Packed Arrays serialize as a generic Array once a value inside is edited, both in key and value.

I had some personal issues, but this is up to discussion as others may not find them significant:

• I'm not sure about aligning the value to the right in key edit mode. It is to the left in all property editors, so feels a bit jarring;
• I have an issue with Edit Key being in the context menu - other props don't have anything there except copy+paste so it's less discoverable. Perhaps a simple left-click could change it too, as it's not bound to any action?

Overall, great job! All stable so far in my testing (might test it more extensively later). As someone who was so annoyed as to try fix it via addon, I'm really looking forward to seeing this in core!

[adamscott]

Can you show pictures of your implemented feature in the description?

[ajreckof]

• Types with a bottom editor (Vec3, Vec4, Plane, Quat...) show at the bottom in both modes;

I was aware of this one but I do not know how to make it more clear. This was one of the reasons i kept the text right aligned because without it, it would be even worst aas their would be no difference at all. If you have any idea on how to make it clearer, I would be really interested.

• Only some types are kept when adding an entry: String, StringName, NodePath, Array, Dictionary, Object revert to null both in value and key;

forgot to reinitialize the type after zeroing it is fixed now

• Packed Arrays serialize as a generic Array once a value inside is edited, both in key and value.

This bug gave me heavy sweat but once i finally debugged it. I found out it wasn't a bug I introduced. But a bug that was already there I'll make a separate issue and PR for this one as it is more a bug of EditorPropertyArray than EditorPropertyDictionary.

Can you show pictures of your implemented feature in the description?

I added some picture and a small video

[don-tnowe]

Re: Types with a bottom editor show at the bottom in both modes
I was aware of this one but I do not know how to make it more clear

I imagined the bottom editor would move to become the "top editor". Like with inline editors having key on the left and value at the right, I imagine with the value staying at the bottom and key at the top.

Though I wonder if it's even possible to place the bottom editor before the inline. If it's not, then aligning the label to the right would indeed be a good workaround. Or could even stay like that either way: on my mock-up above the key+value pair isn't as clearly in key-edit mode (only the "edit" button hints at the connection). For comparison, both:

[ajreckof]

That would be effectively be pretty good but I'm not sur how to do that, to revert from left to right I hijacked the right to left functionality that was developed for language support

[adamscott]

That would be effectively be pretty good but I'm not sur how to do that, to revert from left to right I hijacked the right to left functionality that was developed for language support

What's the behavior for right to left languages, then?

[ajreckof]

What's the behavior for right to left languages, then?

I haven't implemented it yet but it is in my to do list on my personal note but haven't put it here as it is a simple fix I will fix with the next batch of update with the fix to a bug i found about wrong interaction between deleting an item and the edition of another one. The idea is just to change to the direction other than Locale and then put every node in locale direction. I wasn't sure how i should implement it so i reported it to later. I'm wondering wether i should detect what is local and set LTR or RTL based on it or if i should create a new constant being always the other direction from locale.

Edit for a quick view of how it would look like I've implemented the LAYOUT_DIRECTION constant way:

[DukeVengeance]

Thanks for the PR, can we unify the value editing process like Array? i.e. Click Add Key/Value Pair first and then Edit the values, Dictionary is the only place that requires editing values first then click that button, if one forget to click and then the content is lost.

As shown in multiple places as well as godotengine/godot-proposals/#5775, Editing values only to forget to click Add Value seems to be pretty common.

[ajreckof]

I just had an idea for that while trying to explain what I was thinking to reduce this problem. Tell me what you think about it. My new would i think mostly resolve the problem.
The idea would be to keep the possibility to modify values before adding but to also have the ability to add invalid keys (already present or null) those invalid key items would be added and would automatticaly edit the key. As their would be no fallback, I think the item should be removed if the key is not set to a valid value before closing the dictionary's editor. Moreover the edit value option should be deactivated as it will not be possible.
As the dictionary editor is right now it would not enables you to add multiple elements with invalid keys.