[PATCH] Groups

[Badmadblacksad]

• Create groups (Ctrl+number)
• Add to group (Shift+number)
• Select group (number or click on icons)
• A unit can only be a member of a single group

copy ​this image in binaries/data/mods/public/art/textures/ui/session/icons/sheets/groups.png (created with binaries/data/mods/public/art/textures/ui/session/icons/single/group[0-9].png)

On my computer I had to replace hotkey.selection.group.[0-9] with hotkey.selection.group.Num[0-9] in the file binaries/data/config/default.cfg. I've got a french keyboard. Don't know if it will work for you.

TODO :

• double click / double press on a key : snap
• maybe some art related stuff ;)

[Kieran P]

Moving to alpha 5. If the code is finished and works well, and Philip gets time to review before release and commit it, I don't see a reason why it couldn't be in A5.

[historic_bruno]

Creating and selecting a group seems to work.

I can't seem to add to a group, pressing Alt+number only brings up the original grouping for me.

Not sure how much I like the group icons being permanently shown, but others may have different opinions on that. I can see how they would be useful.

I think there should be some indication of which group a selected unit is in, perhaps showing an icon on the details panel? (although it looks like the "group" icon is already in use to show how many units are in a selection, maybe an artist will need to come up with another design :))

[Badmadblacksad]

hum.. it's a mistake in the description : press Shift+number to add a unit to a group.

Alt+number do nothing right now, but it will select a group and follow it (center the camera on its first unit) if you replace

//@todo snap

with

Engine.CameraFollow(toSelect[0]);

in the file binaries/data/mods/public/gui/session/input.js, line 1103. don't think it worth a new diff :)

[Philip Taylor]

Thanks, this seems to mostly work nicely, but I have a few randomly-ordered comments/questions:

performGroup shouldn't use HotkeyIsPressed, because it will fail if the player holds the save/add hotkey then presses a digit then releases save/add before the next GUI tick calls HotkeyIsPressed (which will happen sometimes at low framerates). I think it would probably work better with separate hotkeys like

hotkey.selection.group.select.0 = 0
hotkey.selection.group.add.0 = "Shift+0"
hotkey.selection.group.save.0 = "Ctrl+0"

etc, so it uses the modifier keys at the time when the digit was pressed. (Hopefully our hotkey system will work for that without any problems, but I could be mistaken.)

I thought snap-to-group was typically done in RTS games by double-pressing the group hotkey, rather than by holding yet another modifier key, and I'd expect that to be more intuitive, so it might be best to remove hotkey.selection.group.snap entirely (unless people disagree).

Rather than adding loads of <object hotkey="selection.group.0">, it'd probably be cleaner to have handleInputAfterGui check for ev.type == "hotkeydown" && ev.hotkey.indexOf("selection.group.") == 0) or something along those lines.

The numbers on the group icons seem very hard to read. Might be better to use a non-numbered shield texture, with the number drawn as text on top in a proper font, like the unit count in the selection details panel when you select multiple units.

The tooltips on the group icons should say something more appropriate (maybe "Click to select grouped units").

updateGroups only gets called once per simulation turn, so there's a delay before updating the display when the user changes group assignments. It probably should trigger updateGroups immediately whenever the group assignments are changed.

Why are groups named ("group-1" etc), instead of just using numeric indexes? (Are there some non-numbered groups we're likely to want to add support for in the future?)

When the player saves the game, should their group assignments get saved? If so, we'll need to remember to add support for that when implementing saved games.

In addEntities: instead of for (var gent in group.ents) { if (ent == gent) do if (ent in groups.ents) (since that's simpler and faster).

Should remove the console.write.

g_Groups.addEntities(groupName,selection); (and other places) should have a space after , for consistency with the typical coding conventions.

Instead of parseInt(ent) use +ent (which does pretty much the same thing) for consistency.

[Badmadblacksad]

Replying to Philip:

here is a corrected version. don't need to add the group.png image anymore (cannot edit the description), just patch. relevant comments as always.

[Philip Taylor]

Looks good! Noticed some minor bugs:

• Got JS errors when double-pressing a digit which has no group members.
• The unitGroupPanel blocked clicks, so you couldn't select units behind the hidden group buttons. (Fixable by removing the type="image" so it's a non-interactive container).

I've fixed those things and done some minor formatting cleanups etc, and don't see any other problems so I'll commit now.

[philip]

(In [9322]) # Add numbered unit selection groups, based on patch from Badmadblacksad. Fixes #773.