Editor shortcuts are reset on each start

[KoBeWi]

Godot version

66ab3ce

System information

W10

Issue description

When you open editor, all editor shortcuts are default.

Steps to reproduce

1. Open editor settings
2. Mess with shortcuts
3. Close editor
4. Open again
5. Your fancy custom shortcuts are gone

Minimal reproduction project

No response

[KoBeWi]

The issue is gone without a trace. How mysterious 😒

[KoBeWi]

Reopening, something is definitely wrong here:

I can't save this shortcut. Whatever I define, it doesn't appear in editor_settings.tres. Interestingly, the step with revert arrow IS saved, so the editor explicitly saves empty shortcut (even if it's the same as default) and when I revert, it reverts to the shortcut I previously tried to assign and it doesn't get saved. This might randomly happen with any other shortcut. Default value gets messed up and your shortcut won't be saved.

Might be related #53373
Also most likely caused by #51273
CC @EricEzaM

[EricEzaM]

@KoBeWi Yep it is a bug, thanks.

I think #51273 did cause the issue. In that code, the "original" for each shortcut was set as below when it is registered in editor settings. As you can see, the events AND the original (used for change checks and saving) references the same object. So when an event was added to the events array, it was also added to the "original". This is why saving and the revert button were messed up.

sc->set_events(events);
sc->set_meta("original", events);

I think this should be enough to fix it.

sc->set_events(events);
sc->set_meta("original", events.duplicate(true));

[EricEzaM]

This also affected the undo-redo capability of the editor shortcuts. Passing around & storing pointers, man...

When testing the changes above in editor_settings.cpp, there were still issues:

1. Set Shortcut (revert icon would appear)
2. Revert
3. Set shortcut again to same value
4. Revert icon would not longer be there. Clearing the value would show the revert button, but it would be reverted back to the shortcut you pressed, which is not the actual default.

It appears the last culprit is in settings_config_dialog.cpp, where we get the shortcut events and add them as a meta value on the tree item. This is all passed by reference, which of course means that by the time you get to creating the undo_redo action, the shortcut events are already updated since it is the same memory. I'm not sure how this was affecting the value of the "original" array, but fixing this also fixed the issue described in the above steps.