search

EsotericSoftware / spine-editor

Issue tracking for the Spine editor.
http://esotericsoftware.com/
29 stars 2 forks source link

Ctrl/cmd prevents to properly switch tool in edit mesh mode #816

Open davidetan opened 1 month ago

davidetan commented 1 month ago

A user reported on the forum that when setting hotkeys cmd + 1 and cmd + 2 to activate Edit Mesh - Modify and Edit Mesh - Create respectively, a strange behaviour occurs as if the UI updated the tool, but the application state holds the previous tool.

I was able to track down the problem to the keyUp method method of MeshEditMode where lastTool is restored when ctrl/cmd is released. This is because while ctrl/cmd is pressed in MeshEditMode, a different tool is activated, and the previous one has to be restored upon ctrl/cmd release.

The issue is of the same nature as https://github.com/EsotericSoftware/spine-editor/issues/803#issuecomment-2219846313, but in another piece of code.

Should we consider and mark ctrl/cmd not usable for certain hotkeys, or should we find a way to fix the code so that those keys are usable as hotkeys? If we choose the former, we should probably add a validation for those hotkeys that cannot use ctrl/cmd so that when the users reload the hotkeys, a dialog lists the names of the actions associated to the invalid hotkeys.

NathanSweet commented 1 month ago

I haven't had a chance to look at the code yet, but maybe the hotkey that changes the tool should, when ctrl is down, change the tool that will be "restored" when ctrl is released.

davidetan commented 1 month ago

The problem is not strictly connected to the hotkey actually. You can reproduce the bug just by clicking the button while holding ctrl. Users shouldn't do that, but if they do, the application enters in a inconsistent state.

Anyway, I guess the solution is still setting the lastTool properly because right now it restores the lastTool that was stored when ctrl was pressed.

In both cases when the hotkey ctrl + 1 or a ctrl + click on the button, currently is happening this: 1) the keyDown triggered by ctrl stores the current tool (tool 1) into lastTool, then changes to current tool to the tool connected to ctrl (tool 2) 2) when the ctrl + 1 is completed or the click of ctrl + click is performed, the current tool is changed to the desired one (tool 3) 3) when the ctrl is released, the lastTool (tool 1) overwrites the current tool (tool 3) - we ends up having again the initial tool (tool 1).

If I add a if (ctrl()) meshMode.lastTool = null; in the onChange of the buttons, it work. That because at step 2 we changed tool, so we don't want to restore the tool that was present two tools ago at ctrl release.

However, this fix requires to add it to all buttons. Maybe you have a better idea.

NathanSweet commented 1 month ago

That fix sounds pretty OK.

It may be cleaner to have MeshEditMode override Mode setTool, but it is called in many places. The changes needed to make that work are larger, and it's less clear if anything breaks. Probably best to just add that code in each button.