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

synchronize selected layers between tool and add hotkeys #3776

Merged
merged 4 commits into from
Dec 15, 2022

Conversation

thelsing
Copy link
Collaborator

@thelsing thelsing commented Dec 12, 2022

Identify the Bug or Feature request

fixes #3731
fixes #1998

Description of the Change

Moved the LayerSelectionDialog to DefaultTool. Tools are still responsible for displaying it.
The dialog now always sets the active layer in ZoneRenderer. Tool that need to notified of a layer change can overide the selectedLayerChanged method.
(Commented out because if discussion.) I also added CTRL + 1-4 as hotkeys to select layers.

Possible Drawbacks

Nothing.

Documentation Notes

/* Layers can now be changed via CTRL - 1 - 4 */

Release Notes

/* - added CTRL+1-4 hotkeys for layer selection */

  • synchronize selected layer across different tools

This change is Reviewable

@kwvanderlinde
Copy link
Collaborator

Appreciate the synchronization change. Broadly looks good, though can't really look closely at the moment.

But I'm not convinced about the hotkeys, and there hasn't been any discussion on them. My reactions:

  • Is there other software based on a layering paradigm that we could emulate? Thinking of whether there is already a defacto standard keybindings that we could leverage. Are these binding that?
  • There is a competing idea for using CTRL+number for token grouping/selection. Very common in RTS, might be valuable in MT as well, but would be incompatible with these layer hotkeys.
  • How do these hotkeys age if there is a change to layers? E.g., by adding additional layers, either built-in or custom?

@Azhrei
Copy link
Member

Azhrei commented Dec 13, 2022

I believe the Cmd+[1-4] shortcuts should be fine on macOS. (I've checked the docs and I see that Shift+Cmd+[1-5] are used.)

In regards to future layer expansions, I would expect that Meta+1 moves to the Background layer, and if there are multiple, then repeated Meta+1 keystrokes would cycle through them. If/when additional layers are added, the keystrokes would/could change, and that could be very confusing! I picture a new "Foliage" layer that is between the GM/Hidden layer and the Object layer, for example. (Yeah, silly layer. But that's not the point. ;) )

I would like to see the hockey changes separated from the other functionality of this patch. That allows for more discussion while getting the basic refactoring ready and in-place.

@FullBleed
Copy link

FullBleed commented Dec 13, 2022

Can we get a function to change layers, too?

Some of us hate hotkeys... especially when we don't have the ability to change the default keybindings.

Which brings me to my second request... which should probably be a Feature Request on its own (but seems relevant to bring up here)... Whats the feasibility of allowing us to configure our own key-bindings? I know when that option exists in other apps I almost always go in and make changes. I might, for example, just make the up and down arrows change layers in MT. I don't use them for anything else and it's a single key option.

@thelsing
Copy link
Collaborator Author

Can we get a function to change layers, too?
There is #3708 for this. I'm not changing any macro stuff for now.

I think users really should be able to to change hotkeys. This is part of #3767

I'll disable the hotkeys for now.

Disable hotkey setting for layer selection.
@Phergus Phergus merged commit 96d07a1 into RPTools:develop Dec 15, 2022
@thelsing thelsing deleted the feature-layerselection branch December 22, 2022 22:49
@cwisniew cwisniew added the feature Adding functionality that adds value label Mar 29, 2023
@Azhrei
Copy link
Member

Azhrei commented Apr 1, 2023

I saw this was merged. We'll need the wiki updated, probably with a note that it applies to v1.13+. Thanks @thelsing for this. :)

@aliasmask
Copy link

Will players still be able to bind macros with those hot keys, or have they been removed from the possible choices?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value
Projects
Status: Merged
7 participants