bugfix: Attacking infantry no longer attempt to path to their targets when force-evacuated from a vehicle - #3098
Conversation
… when force-evacuated from a vehicle
|
Can you check if there's issue for this? 1337 perhaps. |
|
Yeah, this looks to be very similar to 1337 indeed. If this pull also fixes the unresponsiveness to player commands it should be identical. From your description it seems like it is. Based on that, I would highly recommend putting this change behind the |
xezon
added
China
| m_goalWaypoint = nullptr; | ||
| m_goalSquad = nullptr; | ||
|
|
||
| #if !RETAIL_COMPATIBLE_CRC |
There was a problem hiding this comment.
Better also put it behind a PRESERVE_... macro.
There was a problem hiding this comment.
I didn't do this because the temporary state reset logic is not necessarily exclusive to the issue at hand. I could make it more broad/generic and have something like PRESERVE_UNINTERRUPTIBLE_TEMP_AI_STATES, but that's not really indicative of the bug it's targeting.
|
|
||
| #if !RETAIL_COMPATIBLE_CRC | ||
| if (m_temporaryState) | ||
| m_temporaryState->onExit(EXIT_RESET); |
There was a problem hiding this comment.
Any idea if this could also affect some other ai behaviors? State is really vast, used by many different things.
There was a problem hiding this comment.
The only other setTemporaryState cases are AI_FOLLOW_EXITPRODUCTION_PATH (only when AI player exits an attacked Tunnel), AI_MOVE_OUT_OF_THE_WAY, and several AI_BUSY cases in the deploy logic. I've been unable to find any issues or regressions with these states.
Stubbjax
added
Committee2026
|
I accidentally just discovered that exiting the temporary state when given new commands also seems to fix #199. BeforeBEFORE.mp4AfterAFTER.mp4 |
|
/agentic_review |
Stubbjax
added
Buff
Code Review by Qodo
1. Evacuation fix behind RETAIL_COMPATIBLE_CRC
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| #if !RETAIL_COMPATIBLE_CRC | ||
| if (m_temporaryState) | ||
| m_temporaryState->onExit(EXIT_RESET); | ||
|
|
There was a problem hiding this comment.
1. Evacuation fix behind retail_compatible_crc 📎 Requirement gap ≡ Correctness
The new evac/command-responsiveness cleanup that clears m_temporaryState / the AI goal object is wrapped in #if !RETAIL_COMPATIBLE_CRC, but this repo defaults RETAIL_COMPATIBLE_CRC to 1, so default (retail-compatible) builds won’t include the fix and the reported bug may remain. This risks failing the requirement that infantry immediately obey attack commands after emerging from destroyed fireport vehicles.
Agent Prompt
## Issue description
The responsiveness bugfix is currently compiled out in the default configuration because the new cleanup/goal-reset logic is guarded by `#if !RETAIL_COMPATIBLE_CRC` while `RETAIL_COMPATIBLE_CRC` defaults to `1` in this repo, so retail-compatible/default builds won’t contain the fix.
## Issue Context
PR Compliance ID 2 requires infantry to respond immediately to player attack commands after emerging from destroyed fireport vehicles. The PR introduces new cleanup behavior (clearing `m_temporaryState` / resetting temporary state exit and clearing the AI goal object during `scatter`) intended to restore responsiveness, but because it is behind `!RETAIL_COMPATIBLE_CRC` it only takes effect when explicitly building with retail CRC compatibility disabled. This affects both Generals and GeneralsMD implementations of `AIStateMachine::clear()` and `OpenContain::scatterToNearbyPosition()`, and the CMake config referenced does not appear to override the default macro value.
## Fix Focus Areas
- Core/GameEngine/Include/Common/GameDefines.h[90-96]
- cmake/config-build.cmake[70-74]
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1012-1029]
- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1021-1033]
- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1024-1029]
- Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[669-679]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[768-775]
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[771-774]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (m_temporaryState) | ||
| m_temporaryState->onExit(EXIT_RESET); | ||
|
|
||
| m_temporaryState = nullptr; |
There was a problem hiding this comment.
2. Temporary state double-exit 🐞 Bug ☼ Reliability
When !RETAIL_COMPATIBLE_CRC is enabled, AIStateMachine::clear() calls m_temporaryState->onExit(EXIT_RESET) after StateMachine::clear() already exits the current state; because states are singletons per StateID, setting a temporary state to the current StateID would invoke onExit() twice on the same State instance.
Agent Prompt
## Issue description
`AIStateMachine::clear()` now exits the temporary state after calling `StateMachine::clear()`. If `m_temporaryState` aliases the state that was current (same StateID), the same `State` object can receive `onExit(EXIT_RESET)` twice.
## Issue Context
`StateMachine` stores exactly one `State*` per `StateID` and returns that same pointer from `internalGetState()`. Both `setState()` and `AIStateMachine::setTemporaryState()` use this mechanism, so same-ID implies same instance.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1012-1029]
- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[1021-1033]
- Generals/Code/GameEngine/Source/Common/StateMachine.cpp[344-363]
- Generals/Code/GameEngine/Source/Common/StateMachine.cpp[483-525]
- Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp[914-961]
## Suggested change
- Capture `StateID oldCurrent = getCurrentStateID();` before calling `StateMachine::clear()`.
- After `StateMachine::clear()`, only call `m_temporaryState->onExit(EXIT_RESET)` if `m_temporaryState != nullptr` AND `m_temporaryState->getID() != oldCurrent`.
- Always set `m_temporaryState = nullptr` (and optionally reset `m_temporaryStateFramEnd = 0`) afterward.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
5 participants
This change fixes an issue where infantry would attempt to path to their targets when force-evacuated from a vehicle.
Force-evacuated infantry would incorrectly retain their goal object reference, which would override the path/destination the infantry were given upon evacuation to that of their attack target instead of their scatter position. Furthermore, temporary AI states would not be cleared when the state machine was cleared upon receiving new commands, causing subsequent commands to effectively provide the temporary state with new data (i.e. giving an attack command during the scatter state would just assign a goal object and cause the unit to path to the target).
The fix is to clear the goal object when infantry are scattered as well as any temporary states when the main state machine is cleared (usually when receiving a new command). As a result, evacuation behaviour is consistent across all vehicles under all circumstances and units feel nice and responsive.
Before
The force-evacuated Tank Hunters attempt to path to the Emperor Overlord
BEFORE.mp4
After
The force-evacuated Tank Hunters are correctly scattered
AFTER.mp4