beyond-all-reason / beyond-all-reason/RecoilEngine

Category filter for /select does not work

Open
#816 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
679
Forks
290
Avg merge
3d 2h
Merged PRs (30d)
40

Description

In BAR, the command `/select AllMap+_Category_MOBILE+_ClearSelection_SelectAll+` should, in theory, replace the current selection with one containing all mobile units owned by the player, but it does not. Instead, it clears the current selection. In Zero-K the `MOBILE` category also exists and the mentioned `/select` command does not match it correctly either.

The relevant code for the filter is here:
https://github.com/beyond-all-reason/spring/blob/782985a8e518d33b5af0636dbb078585967e214a/rts/Game/UI/SelectionKeyHandler.cpp#L186-L192
It tests `unit->category == cat`, which I think is incorrect. `unit->category` is a bitset containing the entire list of categories that apply to the unit, which means the filter tests whether the unit is in the candidate category *and no other categories*, rather than testing whether the set of categories for the unit contains the candidate category, which would be much more useful.

Units with only one category are very rare in BAR and Zero-K, though some do exist in Zero-K at least, which is useful for testing. In Zero-K, the Hacksaw turret only has the `FLOAT` category, for instance. Sure enough, `/select AllMap+_Category_FLOAT+_ClearSelection_SelectAll` selects all the player's Hacksaws, alongside other immobile units whose only category is `FLOAT`.

Though I haven't tested this, I expect changing `unit->category == cat` to `(unit->category & cat) != 0` would fix this bug.

Also, it looks like the `Category` filter could be extended to support multiple categories just by using `(unit->category & cat) == cat`, and `GetCategories()` instead of `GetCategory()`. Aside from the filter being largely non-functional, this is already possible by chaining `_Category_CATA_Category_CATB`, but this is clumsier to write and not as performant, though the performance impact should be negligible.

Contributor guide

Open the contributing guide

Research direction

Start in rts/Game/UI/SelectionKeyHandler.cpp around lines 186-192, then reproduce the issue with the BAR MOBILE and Zero-K FLOAT /select commands. Verify the category filter against units with multiple categories and confirm that the intended behavior, including any multi-category handling in the issue, is covered by tests or repeatable in-game checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
game-dev
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.