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

Add hotkey for removing selected actors from control groups #15990

Merged

Conversation

@dragunoff
Copy link
Contributor

commented Jan 2, 2019

In this PR:

  • Add RemoveFromControlGroup hotkey (defaults to Unknown)
  • Add RemoveFromControlGroup method to OpenRA.Game.Selection

The default hotkey is up for discussion but I think it's a good choice because it's on the same row as the numbers, is easy to reach and is functions similarly to how control groups are assigned.

Closes #15961

@pchote

This comment has been minimized.

Copy link
Member

commented Jan 3, 2019

I'd like to see other people's opinions on this, but IMO we shouldn't add this - I don't think any of the original games had a specific hotkey for this, and the same function can be achieved by adding the unit to a different group.

Every new hotkey type adds an overhead for players to remember, and a maintenance burden / opportunity cost for us (reducing the number of keys available for other things). Also note that ` in particular moves around on various international keyboard layouts, and when paired with Cmd on macOS (all Ctrl shortcuts are remapped to Cmd) it doesn't seem to work.

@MustaphaTR

This comment has been minimized.

Copy link
Member

commented Jan 3, 2019

I dunno if our shortcut code actually look for the keys, or the place on the keyboard. But just to note, in Hungarian Keyboard, there is 0 to the left of 1. Maybe CTRL+ Backspace can be a better idea if this ever gets in.

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2019

Most people will have to use both hands to hit these keys. If we look back at the original issue, time and control have been the main factors that motivated the author to ask for this feature. In his opinion, re-assigning the control-group would create a greater "mess" than removing certain units from a control-group. I know that these settings could be changed to a more ergonomic setup by the player, however these defaults alone make me doubt if there is any additional value in this change.

The author raised another point that is more interesting IMO: Removing units from control-groups when they are sent to repair. It is the underlying problem this feature request tries to address. You have to remove those units from a control group, that you want to send to repair. Otherwise any order given to the control-group will "overwrite" the repair order.

IMO, there is no problem with assigning control-groups (which includes the removal of units from control-groups). However - and this would need to be discussed separately - I would support a change that removes units (temporarily?) from control groups, when they are send to repair.

MustaphaTR is right that a unit can only be in one control group. This means that you already have a very fast and more comfortable solution to remove single units from conrol-groups by assigning a "dummy" control-group to them. In the light of this, I don't see additional value in this change that justifies the costs pchote already mentioned.

@rautamiekka

This comment has been minimized.

Copy link

commented Jan 3, 2019

Been dreaming about this for a very long time. Not a single game allows to drop an unit from a group, and it's indeed more of a mess to reassign than drop since those units will still involve themselves when they shouldn't.

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2019

@pchote Those are all valid points and things that I've been thinking about as well. I agree we need more opinions here. Here's my opinion:

Overhead for players
I think this is not a big problem as players don't bother remembering all of the hotkeys but only the ones that they find useful. For example there a lot of keys that I don't use - viewport bookmarks, production facilities cycle and player stance toggle (I wish there was a way to unbind them but that's another issue).

Any piece of software with a complex UI will probably have a lot of hotkeys (think of photo editing suites, softrawe IDEs, etc.) and novice users will perform most tasks first with the mouse, gradually learning the hotkeys and deciding which ones make sense to their workflow (playstyle).

Maintenance cost
This is a fairly simple hotkey.

The need for this hotkey
This is the most important part of the question... The most common use-case is for units that are ordered to repair. Other use-cases will depend on player style - a few from my experience: splitting a small part of a grouped army, managing air units (I tend to use 2-3 control groups for aircraft).

Perhaps this is not sufficient to justify the existence of such a hotkey. And as mentioned there is a work-around (adding to a dummy group) but to me that feels awkward.

@abcdefg30
Copy link
Member

left a comment

I agree with Mustapha that ctrl + backspace might be a better default. (As it should work across all systems? Plus backspace fits to removing an item.)
I think this feature can be really useful. (Especially with aircraft splitting up for rearming.)

@dragunoff dragunoff force-pushed the dragunoff:feature/remove-from-control-group-hotkey branch from 18df128 to ccf2ae9 Mar 11, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2019

Rebased and updated.

  • Default hotkey changed to Ctrl + BACKSPACE
  • Shorten hotkey description (it almost fits in the UI now, I'm open to suggestions for better wording)

@dragunoff dragunoff force-pushed the dragunoff:feature/remove-from-control-group-hotkey branch from ccf2ae9 to f220744 Mar 12, 2019

@@ -18,6 +18,10 @@ SelectUnitsByType: W
Description: Select units by type
Types: World

RemoveFromControlGroups: BACKSPACE Ctrl
Description: Remove from control groups

This comment has been minimized.

Copy link
@obrakmann

obrakmann Mar 26, 2019

Contributor

Please make it "Remove from control group", otherwise the text overflows in the dialog. Also please leave the default hotkey empty, as a compromise. 👍 after that.

This comment has been minimized.

Copy link
@pchote

pchote Mar 26, 2019

Member

Yeah, i'm happy to let my objections slide if this becomes an option feature that players enable themselves.

This comment has been minimized.

Copy link
@dragunoff

dragunoff Mar 27, 2019

Author Contributor

Changes applied.

Add hotkey for removing actors from control groups
* Add `RemoveFromControlGroup` hotkey
* Add `RemoveFromControlGroup` method to `OpenRA.Game.Selection`

@dragunoff dragunoff force-pushed the dragunoff:feature/remove-from-control-group-hotkey branch from f220744 to 8780751 Mar 27, 2019

@dragunoff

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

Rebased and applied requested changes.

@obrakmann obrakmann merged commit 7695714 into OpenRA:bleed Mar 28, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@obrakmann

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

@dragunoff dragunoff deleted the dragunoff:feature/remove-from-control-group-hotkey branch Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.