-
Notifications
You must be signed in to change notification settings - Fork 145
BugFix(Pathfinding): Fix late game unit lockups and impassable terrain #2140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…Cell header to the cpp (#)
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Include/Common/GameDefines.h | Added new RETAIL_COMPATIBLE_PATHFINDING_ALLOCATION define to control pathfinding memory allocation mode |
| GeneralsMD/Code/GameEngine/Include/GameLogic/AIPathfind.h | Moved obstacle data from PathfindCellInfo to PathfindCell, changed inline functions to declarations for retail failover support |
| GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIPathfind.cpp | Implemented dual-mode pathfinding with failover: tracks obstacles in PathfindCell to survive resource exhaustion, cleans up orphaned cells during failover |
Sequence Diagram
sequenceDiagram
participant Game
participant Pathfinder
participant PathfindCell
participant PathfindCellInfo
Note over Game,PathfindCellInfo: Normal Operation (Retail Mode)
Game->>Pathfinder: Place building on map
Pathfinder->>PathfindCell: setTypeAsObstacle(obstacle)
PathfindCell->>PathfindCell: Set m_obstacleID, m_obstacleIsFence, etc
PathfindCell->>PathfindCellInfo: getACellInfo()
alt Resources Available
PathfindCellInfo-->>PathfindCell: Return info
PathfindCell->>PathfindCellInfo: Store obstacle data in m_info
else Resources Exhausted
PathfindCellInfo-->>PathfindCell: Return nullptr
Note over PathfindCell: Cell becomes orphaned<br/>(has m_obstacleID but no m_info)
end
Note over Game,PathfindCellInfo: Failover Trigger
Game->>Pathfinder: Request pathfind
Pathfinder->>PathfindCell: Process cells
PathfindCell->>PathfindCell: Detect nullptr m_info
PathfindCell->>Pathfinder: Set s_useFixedPathfinding = true
Pathfinder->>Pathfinder: forceCleanCells()
Pathfinder->>PathfindCell: isObstructionInvalid()
alt Orphaned Cell Found
PathfindCell-->>Pathfinder: true (has m_obstacleID, no m_info)
Pathfinder->>PathfindCell: clearObstruction()
PathfindCell->>PathfindCell: Clear m_type, m_obstacleID
end
Note over Game,PathfindCellInfo: Fixed Pathfinding Mode
Game->>Pathfinder: Place/remove building
Pathfinder->>PathfindCell: setTypeAsObstacle() / removeObstacle()
PathfindCell->>PathfindCell: Use static m_obstacleID fields only
Note over PathfindCellInfo: PathfindCellInfo bypassed<br/>No resource exhaustion possible
Greptile's behavior is changing!From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
|
Fish tested the failover mechanism, when the game is running before failover you can make areas of the map impassable, after the failover those areas were properly cleared and passable again. The game also allowed units to properly move again. compared to before the fail over, more buildings could be placed without causing unit lockups. |
|
Does not fix #424, with |
Okay there must be something else happening there, could be due to AI State handling instead of pathfinding. |
Merge by rebase
Closes: #1984
This PR come in two parts, a retail compatible version, with a failover when the pathfinding crashes, and a non retail compatible path.
The new define
RETAIL_COMPATIBLE_PATHFINDING_ALLOCATIONwas added and will be used for further pathfinding changes that are not retail compatible that will be upcoming. These changes alter data handling within the pathfinding.The main issue resolved is when units stop moving late game and where parts of the map become impassable.
The reason units stop moving is that objects / buildings placed on the map use up PathfindCellInfo resources which are in a limited pool and are used during the A* Pathfinding.
In this PR we first refactor the obstacle handling functions within PathfindCell from the header and into the cpp, this was required to allow the retail compatible changes to make use of the static variable that signals the pathfinding failover within the cpp file.
In the second commit, the objectID, object blocked by ally, object is transparent and object is fence variables were added to the PathfindCell from the PathfindCellInfo. These variables are then used explicitly in the non retail codepath, in the retail mode they are used in parallel before failover to help identify invalid impassable terrain, then used in a dedicated way in the fixed pathfinding pathway.
Might need better names on the commits but can discuss it bellow