feat(option): Add user option to enable or disable Right Mouse Scroll with Alternate Mouse#2798
Conversation
… with Alternate Mouse Toggle with UseRightMouseScrollWithAlternateMouse=yes/no in Options.ini
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/Common/OptionPreferences.h | Adds getRightMouseScrollWithAlternateMouseEnabled() const declaration, consistent with existing getter style. |
| Core/GameEngine/Source/Common/OptionPreferences.cpp | New getter accesses TheGlobalData->m_useRightMouseScrollWithAlternateMouse, but this field is missing from Generals/GlobalData.h, causing a build failure for the Generals target. |
| GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h | Adds m_useRightMouseScrollWithAlternateMouse field to Zero Hour's GlobalData struct correctly. |
| GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp | Initializes and loads the new preference; contains a dead #if RTS_GENERALS branch (never true in ZH builds) and a newly added comment referencing a 2003 date. |
| GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp | Round-trips the preference value on save (no UI control yet, acknowledged via @todo). This is intentional but the setting is effectively read-only from the UI until a checkbox is added. |
| GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp | Correctly gates RMB scroll on the new m_useRightMouseScrollWithAlternateMouse flag using clear boolean logic. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["MSG_RAW_MOUSE_RIGHT_BUTTON_DOWN"] --> B{"m_useAlternateMouse?"}
B -- No --> C["userWantsRMBScroll = true"]
B -- Yes --> D{"m_useRightMouseScrollWithAlternateMouse?"}
D -- Yes --> C
D -- No --> E["userWantsRMBScroll = false"]
C --> F{"isSelecting or isScrolling?"}
F -- No --> G["setScrolling(SCROLL_RMB)"]
F -- Yes --> H["No action"]
E --> H
subgraph Init["Startup: parseGameDataDefinition"]
I["OptionPreferences::getRightMouseScrollWithAlternateMouseEnabled()"]
I --> J{"Key in Options.ini?"}
J -- No --> K["Fall back to GlobalData default\n(Generals=false, ZH=true)"]
J -- Yes --> L["Parse yes/no"]
L --> M["Set TheWritableGlobalData->m_useRightMouseScrollWithAlternateMouse"]
K --> M
end
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
Core/GameEngine/Source/Common/OptionPreferences.cpp:206-216
**Missing `m_useRightMouseScrollWithAlternateMouse` in Generals GlobalData.h**
`getRightMouseScrollWithAlternateMouseEnabled()` accesses `TheGlobalData->m_useRightMouseScrollWithAlternateMouse`, but the field was only added to `GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h`. The Generals build compiles this `Core` file against `Generals/Code/GameEngine/Include/Common/GlobalData.h`, which does not declare this member — causing a hard compilation failure for the Generals target. `Generals/Code/GameEngine/Include/Common/GlobalData.h` needs the new field, and `Generals/Code/GameEngine/Source/Common/GlobalData.cpp` must both initialize it in the constructor and apply the user preference in `parseGameDataDefinition`.
### Issue 2 of 3
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp:1056-1062
The `#if RTS_GENERALS` branch is dead code in this file — `RTS_GENERALS` is defined only for the vanilla Generals build, never for the GeneralsMD (Zero Hour) build. Only the `#else` branch will ever execute here, so the conditional and its comment add noise without effect. The Generals-specific default belongs in `Generals/Code/GameEngine/Source/Common/GlobalData.cpp`.
```suggestion
m_useAlternateMouse = FALSE;
m_useRightMouseScrollWithAlternateMouse = TRUE;
```
### Issue 3 of 3
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp:1058-1059
**Newly added comment references a 2003 date**
The comment `// disable mouse scrolling in alternate mouse mode, per Harvard 7/15/03` is newly introduced in this PR and cites a date from 2003. Per the project's annotation convention, newly created comments should not reference years prior to the current year (2026).
Reviews (1): Last reviewed commit: "feat(option): Add user option to enable ..." | Re-trigger Greptile
| if (stricmp(it->second.str(), "yes") == 0) { | ||
| return TRUE; | ||
| } | ||
| return FALSE; |
There was a problem hiding this comment.
return stricmp(it->second.str(), "yes") == 0;if statement not needed.
There was a problem hiding this comment.
I agree we should simplify the code here, but currently this is consistent with how the other functions in this class are written.
| m_useRightMouseScrollWithAlternateMouse = TRUE; | ||
| #endif | ||
| m_clientRetaliationModeEnabled = TRUE; //On by default. | ||
| m_doubleClickAttackMove = FALSE; |
There was a problem hiding this comment.
No. I saw the nearby mouse input vars are uninitialized so I added init values for them.
Do you need me to remove that?
This change adds a user option to enable or disable Right Mouse Scroll with Alternate Mouse. It can be toggled with
UseRightMouseScrollWithAlternateMouse=yes/noin the Options.iniThe reason for this option is that before merge #2794 the RMB scroll was disabled in Generals, but enabled in Zero Hour. It is now default disabled in Generals and default enabled in Zero Hour, but both games can toggle it. This avoids potential irritations for legacy Generals players with Alternate Mouse setup.
TODO