Skip to content

feat(client): Add EasyMilitaryDrag option to leave builders out of drag selections - #3219

Open
triatomic wants to merge 3 commits into
TheSuperHackers:mainfrom
triatomic:qol/easy-military-drag
Open

feat(client): Add EasyMilitaryDrag option to leave builders out of drag selections#3219
triatomic wants to merge 3 commits into
TheSuperHackers:mainfrom
triatomic:qol/easy-military-drag

Conversation

@triatomic

Copy link
Copy Markdown

Adds an opt-in client option dropping builders from drag selections. Defaults off; retail behavior unless set in Options.ini.

EasyMilitaryDrag (= Yes) — boxing over your base picks up the army without dragging workers off their jobs. Covers KINDOF_DOZER and KINDOF_IGNORES_SELECT_ALL — the same kinds Select All already disqualifies (which also catches GLA Workers). Only drags are affected: point selection, double click, control groups and select-matching behave as before.

Holding Ctrl inverts the filter for that one drag — boxing only the builders — and cancels force-attack mode for that selection, since Ctrl drives both. A box holding only builders still selects them via a second pass with the filter suspended, tested against "no units survived" rather than list emptiness so a building in the box cannot silently win over the workers beside it.

Works in both Zero Hour and Generals — everything lives in the shared selection code.

This option ships in the Contra mod's engine fork and has been played there; this PR is the port onto current main with both follow-up fixes folded in. Code was written with LLM assistance and human-reviewed, adapted and playtested by the author.

Prepared for Squash and Merge.

…ctions

Options.ini: EasyMilitaryDrag = Yes drops builders from a drag selection,
so boxing over a base picks up the army without dragging workers off
their jobs. Covers KINDOF_DOZER and KINDOF_IGNORES_SELECT_ALL - the same
kinds Select All already disqualifies - which also catches GLA Workers.
Only drags are affected: point selection, double click, control groups
and select-matching all behave as before.

Holding Ctrl inverts the filter for that one drag, boxing only the
builders, and cancels force attack mode for that selection since Ctrl
drives both. A box holding only builders selects them after all via a
second pass with the filter suspended - tested against units surviving,
not list emptiness, so a building in the box cannot silently win over
the workers standing next to it.

Works in both Zero Hour and Generals: everything lives in the shared
selection code.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add EasyMilitaryDrag builder filtering to box selection

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Adds opt-in drag filtering that excludes dozers and Select All-ignored units.
• Lets Ctrl-drag select only builders without triggering force attack.
• Retries unfiltered when normal filtering leaves no non-structure units.
Diagram

graph TD
  OPT["Options.ini"] --> PREF["Option preferences"] --> PICK["Drag state"] --> FILTER{"Builder filter"} --> RESULT["Selection result"]
  FILTER -- "No units" --> FALLBACK["Fallback pass"] --> RESULT
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a drag-filter mode enum
  • ➕ Makes normal, inverted, and suspended states mutually exclusive
  • ➕ Could pass one resolved mode through filtering and force-attack checks
  • ➖ Requires a broader selection-state refactor
  • ➖ Still needs careful handling because point-selection state is assigned after construction

Recommendation: Keep the PR’s localized boolean-state approach for this port because it minimizes risk in shared legacy selection code and preserves retail defaults. A dedicated drag-filter enum would be cleaner if selection state is later refactored, but it is not necessary for this focused feature.

Files changed (5) +143 / -0

Enhancement (3) +127 / -0
SelectionInfo.hDefine per-drag builder filtering state +17/-0

Define per-drag builder filtering state

• Extends drawable-pick state with normal, Ctrl-inverted, and fallback-disabled filter flags. Declares a helper that distinguishes inverted drag selection from point force-attack behavior.

Core/GameEngine/Include/GameClient/SelectionInfo.h

SelectionXlat.cppRetry builder-only drags without filtering +36/-0

Retry builder-only drags without filtering

• Detects when filtering leaves no non-structure drawable and repeats the region scan with EasyMilitaryDrag suspended. This prevents nearby structures from taking precedence over builders in builder-only boxes.

Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp

SelectionInfo.cppFilter builders and resolve Ctrl-drag behavior +74/-0

Filter builders and resolve Ctrl-drag behavior

• Caches the preference per selection, excludes builder kinds from normal drags, and inverts the filter for Ctrl-drag. It also suppresses force attack for inverted drags while leaving point selection and other selection mechanisms unchanged.

Core/GameEngine/Source/GameClient/SelectionInfo.cpp

Other (2) +16 / -0
OptionPreferences.hExpose the EasyMilitaryDrag preference accessor +1/-0

Expose the EasyMilitaryDrag preference accessor

• Declares a const getter for the new Options.ini preference so shared client selection code can query it.

Core/GameEngine/Include/Common/OptionPreferences.h

OptionPreferences.cppParse EasyMilitaryDrag from Options.ini +15/-0

Parse EasyMilitaryDrag from Options.ini

• Reads the EasyMilitaryDrag key case-insensitively and enables the behavior only for Yes. Missing or other values preserve retail behavior.

Core/GameEngine/Source/Common/OptionPreferences.cpp

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Selections repeatedly reload Options.ini ✓ Resolved 🐞 Bug ➹ Performance
Description
Every PickDrawableStruct construction creates an OptionPreferences, whose constructor
synchronously calls loadFromIniFile(), so every click or drag reparses Options.ini; Ctrl-drag
handling constructs another instance in contextCommandForNewSelection. This puts avoidable file
I/O and parsing directly in the input-selection path and can cause selection latency or disk churn.
Code

Core/GameEngine/Source/GameClient/SelectionInfo.cpp[R101-102]

+		OptionPreferences optionPref;
+		if (optionPref.getEasyMilitaryDragEnabled())
Evidence
The changed selection constructor instantiates OptionPreferences for every PickDrawableStruct,
while the options class constructor immediately loads Options.ini (or the per-instance equivalent).
onMouseLeftClick constructs this pick structure for every point or area selection, and the later
inversion helper creates another options object for Ctrl drags.

Core/GameEngine/Source/GameClient/SelectionInfo.cpp[80-81]
Core/GameEngine/Source/GameClient/SelectionInfo.cpp[85-102]
Core/GameEngine/Source/Common/OptionPreferences.cpp[49-67]
Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp[724-741]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
EasyMilitaryDrag currently constructs `OptionPreferences` during each selection, which reloads and reparses the options file on the input path.
## Issue Context
`OptionPreferences::OptionPreferences()` invokes `loadFromIniFile()`. Both the pick constructor and Ctrl-inversion helper need the same stable client option value.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/SelectionInfo.cpp[72-115]
- Core/GameEngine/Source/Common/OptionPreferences.cpp[49-67]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/GameEngine/Source/GameClient/SelectionInfo.cpp Outdated
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

Adds an opt-in EasyMilitaryDrag preference shared by Generals and Zero Hour, filtering builders from box selections while preserving ordinary selection behavior.

  • Loads and caches the option in each variant’s global data.
  • Supports Ctrl-inverted builder-only drag selection without entering force-attack behavior.
  • Repeats builder-only drags without filtering when no selectable owned unit survives the first pass.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp Adds the fallback selection pass and now aligns its survivor test with downstream ownership and containment filtering.
Core/GameEngine/Source/GameClient/SelectionInfo.cpp Implements normal and Ctrl-inverted builder filtering while coordinating force-attack and existing drawable-selection behavior.
Core/GameEngine/Source/Common/OptionPreferences.cpp Parses the opt-in EasyMilitaryDrag setting with a disabled default.
Generals/Code/GameEngine/Source/Common/GlobalData.cpp Initializes and caches the preference for the Generals runtime.
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp Initializes and caches the preference for the Zero Hour runtime.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Drag[Drag selection] --> Enabled{EasyMilitaryDrag enabled?}
  Enabled -- No --> Normal[Normal selection]
  Enabled -- Yes --> Ctrl{Ctrl held?}
  Ctrl -- Yes --> Builders[Keep builders only]
  Ctrl -- No --> Military[Filter builders]
  Military --> Survivor{Owned uncontained unit survives?}
  Survivor -- Yes --> Select[Complete selection]
  Survivor -- No --> Retry[Repeat without builder filter]
  Retry --> Select
  Builders --> Select
Loading

Reviews (3): Last reviewed commit: "fix(client): Exclude contained riders fr..." | Re-trigger Greptile

Comment thread Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp
…fallback

Addresses both review findings:

- Constructing OptionPreferences reparses Options.ini from disk, and the
  selection path did it on every click or drag, twice with Ctrl held. The
  option now lives in GlobalData next to the other cached client options
  and refreshes with them; the input path reads the cached copy.

- The builders-only fallback tested for any surviving non-structure, but
  a foreign unit in the box satisfies that test and is then rejected by
  the ownership filtering downstream - suppressing the fallback and
  leaving the boxed builders unselected. Only locally controlled units
  count now.
Comment thread Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp Outdated
A rider in a non enclosing container such as the Overlord's bunker is a
visible, locally controlled non structure, but the final selection loop
discards contained objects - so it satisfied the fallback test while
never surviving selection, suppressing the builders-only pass. Count
only uncontained units.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant