Skip to content

bugfix(locomotor): Fix crash in LocomotorStore::reset when map.ini defines a new locomotor - #3633

Open
Okladnoj wants to merge 2 commits into
TheSuperHackers:mainfrom
Okladnoj:okji/fix/locomotor-reset-erase
Open

Okladnoj wants to merge 2 commits into
TheSuperHackers:mainfrom
Okladnoj:okji/fix/locomotor-reset-erase

Conversation

@Okladnoj

@Okladnoj Okladnoj commented Oct 29, 2026 •

Copy link
Copy Markdown

If a map's map.ini defines a locomotor that the game's INI data does not have, it ends up in m_locomotorTemplates as an override template. When the game data is reset, LocomotorStore::reset() deletes such a template and its node but does not advance the iterator, so the next iteration crashes on the erased node. The iterator is now advanced before erasing: erase(it++).

Reproduction: the map "Time To Kill - ZH - by NeoMaurice" with the ContraX Brutal mod.

Stack trace (macOS, arm64)
FATAL: Caught signal 11
CRASH FAULT ADDRESS: 0x10 code 2
CRASH REGISTERS: pc 0x102d02d2c lr 0x102eeb0dc sp 0x16d14c3c0 fp 0x16d14c3e0 x0 0x764549b1f8 x19 0x76454473a0 x20 0x763d4c18d8 x21 0x76454473b8
CRASH FAULT REGION: unmapped, next region 0x102cb0000
CRASH BACKTRACE:
0   GeneralsOnlineZH                    0x0000000102cb9424 _ZL18macosSignalHandleriP9__siginfoPv + 1176
1   libsystem_platform.dylib            0x0000000193b74144 _sigtramp + 56
2   GeneralsOnlineZH                    0x0000000102eeb0dc _ZN14LocomotorStore5resetEv + 60
3   GeneralsOnlineZH                    0x00000001030ccb68 _ZN22SubsystemInterfaceList8resetAllEv + 44
4   GeneralsOnlineZH                    0x0000000102cf2b30 _ZN10GameEngine5resetEv + 152
5   GeneralsOnlineZH                    0x000000010316cd1c _ZN9GameLogic13clearGameDataEb + 324
6   GeneralsOnlineZH                    0x000000010300d85c _ZL32abortStartWithIncompatibleMapMacPKc + 1868
7   GeneralsOnlineZH                    0x000000010300c920 _ZN9GameLogic15tryStartNewGameEb + 22808
8   GeneralsOnlineZH                    0x000000010300ffec _ZN9GameLogic6updateEv + 116
9   GeneralsOnlineZH                    0x0000000102cf319c _ZN10GameEngine6updateEv + 588
10  GeneralsOnlineZH                    0x0000000102cce528 _ZN15MacOSGameEngine6updateEv + 48
11  GeneralsOnlineZH                    0x0000000102cf3254 _ZN10GameEngine7executeEv + 48
12  GeneralsOnlineZH                    0x0000000102cf614c _Z8GameMainv + 200
13  GeneralsOnlineZH                    0x0000000102cb7618 -[GeneralsAppDelegate runGame] + 520
14  libdispatch.dylib                   0x00000001939a8a34 _dispatch_call_block_and_release + 32
15  libdispatch.dylib                   0x00000001939c25a0 _dispatch_client_callout + 16
16  libdispatch.dylib                   0x00000001939e0998 _dispatch_main_queue_drain.cold.6 + 832
17  libdispatch.dylib                   0x00000001939b7b0c _dispatch_main_queue_drain + 176
18  libdispatch.dylib                   0x00000001939b7a4c _dispatch_main_queue_callback_4CF + 44
19  CoreFoundation                      0x0000000193c679cc __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
20  CoreFoundation                      0x0000000193c2965c __CFRunLoopRun + 1980
21  CoreFoundation                      0x0000000193d0382c _CFRunLoopRunSpecificWithOptions + 536
22  HIToolbox                           0x00000001a105c590 RunCurrentEventLoopInMode + 320
23  HIToolbox                           0x00000001a105f91c ReceiveNextEventCommon + 492
24  HIToolbox                           0x00000001a11e2380 _BlockUntilNextEventMatchingListInMode + 48
25  AppKit                              0x000000019852329c _DPSBlockUntilNextEventMatchingListInMode + 292
26  AppKit                              0x00000001981e04fc _DPSNextEvent + 580
27  AppKit                              0x0000000198a58748 -[NSApplication(NSEventRouting) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 780
28  AppKit                              0x0000000198a5826c -[NSApplication(NSEventRouting) nextEventMatchingMask:untilDate:inMode:dequeue:] + 292
29  AppKit                              0x00000001981d390c -[NSApplication run] + 396
30  GeneralsOnlineZH                    0x0000000102cb7ca4 main + 164
31  dyld                                0x0000000193793e80 start + 6688

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Walkthrough

LocomotorStore::reset now advances its iterator when it erases an entry. This change applies to both game variants.

Changes

Locomotor reset

Layer / File(s) Summary
Erase entries and advance the iterator
Generals/Code/GameEngine/Source/GameLogic/Object/Locomotor.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Locomotor.cpp
Both game variants pass a post-incremented iterator to erase in LocomotorStore::reset.

Priority: Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: xezon, bobtista

Merge Risk: Minimal · up to f0ac8

The reset change addresses the reported crash in both game variants; no remaining merge-blocking concern is identified.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the change: a crash fix in LocomotorStore::reset when map.ini defines a new locomotor. The diff changes erase(it) to erase(it++) in that function, which advances the iterator before er…
Description check Passed The description states the cause: a locomotor defined in map.ini becomes an override template, and reset() erases it without advancing the iterator. It names the fix, erase(it++), and gives a reproduc…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the change: a crash fix in LocomotorStore::reset when map.ini defines a new locomotor. The diff changes erase(it) to erase(it++) in that function, which advances the iterator before erasing the current entry. The title is specific and matches the main change.

Full details: Description check

Explanation

The description states the cause: a locomotor defined in map.ini becomes an override template, and reset() erases it without advancing the iterator. It names the fix, erase(it++), and gives a reproduction map and a macOS crash trace that points to LocomotorStore::reset. This relates directly to the changeset.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge.

Summary

Fixes the reset crash when map.ini adds a new locomotor.

  • Locomotor reset moves past an entry before erasing it.

Reviews (2) · Last reviewed commit: "refactor(locomotor): Remove the redundan..." · Reviewed by Greptile

Okladnoj added a commit to OKJID/GameClient that referenced this pull request Oct 29, 2026
…otors on reset

- A locomotor that a map.ini defines but the loaded INI data lacks becomes an override-only template; LocomotorStore::reset deleted it and erased its node without advancing the iterator, so the loop read the freed node and crashed
- Crashed every start of Time To Kill ZH under ContraX Brutal: the incompatible map rollback resets the game data right after map.ini
- erase(it++) instead of it = erase(it), so the same fix builds with the STLport of the VC6 build; both trees, sent to TSH as PR TheSuperHackers#3633
- DEBUG_INFO_MAC names every override-only template when map.ini creates it and when the reset erases it
@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Stability Concerns stability of the runtime Crash This is a crash, very bad labels Oct 29, 2026
Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Locomotor.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Crash This is a crash, very bad Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker Stability Concerns stability of the runtime ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants