bugfix(pathfinder): Fix inaccurate single unit movement destinations when unobstructed

[stephanmeesters]

• Fixes Unit movement destination is inaccurate #247
• Fixes Inconsistencies in timing when evacuation a China Helix or a USA Chinook #1998
• Relates to Chinook turns when evacing from idle position GeneralsGamePatch#1408

When moving units the exact destination will be determined by pathfinding: the Pathfinder::adjustDestination will adjust a given coordinate to account for obstacles. It will however also adjust the destination when there are no obstacles and place it in the middle of a path find cell (as Mauller suspected in the linked issue).

The fix here is to use the original destination when there is no obstruction.

In testing it appears to fix the chinook landing issue (only when the path where it lands is unobstructed).

It should be determined whether the change to group unit movement is desirable.
The change affects single unit movement only. In testing this would negatively impact the feel of moving around a group of units.

---

Chinook landing motion

debug-chinooks.mp4

Demonstrate accuracy relative to mouse click

output.mp4

Pathcells debug for single unit

debug-single.mp4

Pathcells debug for a group of unit

debug-group.mp4

Todo

• Add comment

[Caball009]

This looks related: #1998

[Mauller]

When running a debug build, use CTRL + A to cycle through the pathfinding overlays, they will show you what tiles a unit is covering when deciding the destination. Mode 1 shows the path created, 2 shows obstructions on the map and 3 shows the cells covered by units in their current position and destination.

You need to make sure a unit is not considered to be taking up more pathfinding cells than it originally would, otherwise it will throw off pathing around that unit if you attempt to accurately place a unit where the mouse clicked compared to how the pathfinding grid is laid out.

This "issue" is likely something we don't want to change the behavior of since it can have consequences on pathfinding in various ways.

[stephanmeesters]

@Mauller I have added extra video's with the pathfinding overlays, one for single units and one for a group, please have a look. As far as I can tell there are no additional cells being used. Group movement appears less clumpy.

[stephanmeesters]

@Caball009

See the chinooks video, we can surely say it's related.

To be clear I don't expect this PR to pass at this point, it changes way more than I set out to do with just the linked bug. But the destination snapping to pathcells is problematic in it's own way for sure.

[stephanmeesters]

In the "Pathcells debug for single unit" video we can see the bomb truck in the right bottom corner sliding after the fix so that's at least one new problem and there are probably more. Closing

[stephanmeesters]

I think this one deserves another look.

This change makes the movement behavior of units more predictable, especially the chinooks (see the video).

It may cause other issues but I haven't seen it yet -- in testing it occupied the same number of pathfinding cells as the original, and the sliding issue seen in one of the videos happens in the current pathfinding as well.

[greptile-apps]

Greptile Summary

This PR fixes grid-snap behaviour in Pathfinder::adjustDestination: when the target path cell is unobstructed, checkForAdjust was silently snapping the destination to the nearest cell centre even though no adjustment was needed. The fix passes a local copy to checkForAdjust and, for single human-controlled units in non-retail-compatible mode, discards the snapped result in favour of the original click coordinate.

• The core change wraps the initial center-cell checkForAdjust call in #if RETAIL_COMPATIBLE_PATHFINDING / #else, preserving original behaviour when that flag is 1 (the default) and activating the fix when it is 0. The spiral-search fallback (obstructed path) is unaffected, so the exact destination is only kept when the path is genuinely clear.
• Scope is deliberately limited to isHuman && singleUnit (getGroup()->getCount() == 1) to avoid changing group-move feel, which was validated in testing.

Confidence Score: 5/5

Safe to merge — the fix is gated behind RETAIL_COMPATIBLE_PATHFINDING=0 so it cannot affect existing retail-compatible builds, and the new code path is narrow (single human-controlled unit, unobstructed center cell only).

The change is additive (purely inside an #else branch of an existing macro) and the retail-compatible path is untouched. The non-retail path is logically sound: exact destination is kept only when the spiral search would have been unnecessary (center cell is clear), and group moves fall through to the original snapping behaviour.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameLogic/AI/AIPathfind.cpp	Adds a #if RETAIL_COMPATIBLE_PATHFINDING guard around the center-cell checkForAdjust call so that, in non-retail mode, single human-controlled units keep their exact click destination when the center path cell is unobstructed, instead of being snapped to the grid-cell center.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["adjustDestination(obj, dest, groupDest)"] --> B{RETAIL_COMPATIBLE\nPATHFINDING?}
    B -- "1 (default)" --> C["checkForAdjust(..., dest, ...)\n(original retail path)"]
    C -- "true" --> D["return true\n(dest = grid-snapped cell centre)"]
    C -- "false" --> E["Spiral search"]
    B -- "0 (fixed mode)" --> F["adjustDest = *dest\ncheckForAdjust(..., &adjustDest, ...)"]
    F -- "false" --> E
    F -- "true" --> G{isHuman &&\nsingleUnit?}
    G -- "yes\n(exact destination)" --> H["*dest unchanged\n(original click coordinate)\nreturn true"]
    G -- "no\n(group / AI)" --> I["*dest = adjustDest\n(grid-snapped)\nreturn true"]
    E --> J["checkForAdjust at spiral offset cells\n(dest passed directly)"]
    J -- "found valid cell" --> K["return true\n(obstructed - must adjust)"]
    J -- "exhausted" --> L["return false / fallback"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["adjustDestination(obj, dest, groupDest)"] --> B{RETAIL_COMPATIBLE\nPATHFINDING?}
    B -- "1 (default)" --> C["checkForAdjust(..., dest, ...)\n(original retail path)"]
    C -- "true" --> D["return true\n(dest = grid-snapped cell centre)"]
    C -- "false" --> E["Spiral search"]
    B -- "0 (fixed mode)" --> F["adjustDest = *dest\ncheckForAdjust(..., &adjustDest, ...)"]
    F -- "false" --> E
    F -- "true" --> G{isHuman &&\nsingleUnit?}
    G -- "yes\n(exact destination)" --> H["*dest unchanged\n(original click coordinate)\nreturn true"]
    G -- "no\n(group / AI)" --> I["*dest = adjustDest\n(grid-snapped)\nreturn true"]
    E --> J["checkForAdjust at spiral offset cells\n(dest passed directly)"]
    J -- "found valid cell" --> K["return true\n(obstructed - must adjust)"]
    J -- "exhausted" --> L["return false / fallback"]

Loading

_{Reviews (8): Last reviewed commit: "Process review comments" | Re-trigger Greptile}

[xezon]

The bot has a complaint.

[xezon]

The fix makes sense.

[xezon]

Looking good