Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
davidetan opened this issue Aug 1, 2024 · 3 comments
Open

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

davidetan opened this issue Aug 1, 2024 · 3 comments

Comments

@davidetan
Copy link
Collaborator

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 #803 (comment), 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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants