The Harvester trait and MoveAdjacentTo activity called the pathfinder but had a single source and multiple targets. The pathfinder interface only allows for the opposite: multiple sources and a single target. To work around this they would swap the inputs. This works in most cases but not all cases. One aspect of asymmetry is that an actor may move out of an inaccessible source cell, but not onto an inaccessible target cell.
Searches that involved an inaccessible source cell and that applied this swapping method would therefore fail to return a path, when a valid path was possible. Although a rare case, once good way to reproduce is to use a production building that spawns actors on inaccessible cells around it, such as the RA naval yard. A move order uses the pathfinder correctly and the unit will move out. Using a force attack causes the unit to use the broken "swapped" mechanism in MoveAdjacentTo and it will be stuck.
This asymmetry has been longstanding but the pathfinding infrastructure only sporadically accounted for it. It is now documented and applied consistently. Create a new overload on the pathfinder trait that allows a single source and multiple targets, so callers have an overload that does what they need and won't be tempted to swap the positions and run into this issue.
Internally, this requires us to teach Locomotor to ignore the self actor when performing movement cost checks for these "in reverse" searches so the unit doesn't consider the cell blocked by itself.
- Enforce SA1604 ElementDocumentationShouldHaveSummary.
- Enforce SA1629 DocumentationTextShouldEndWithAPeriod.
- Turn off some rules covered by IDExxxx rules.
- Remaining rules are treated as part of OpenRA style.
Multiple layers of Lazy<T>ness are replaced with
an explicit two-part loading scheme.
Sequences are parsed immediately, without the need
for the sprite assets, and tell the SpriteCache
which frames they need. Use-cases that want the
actual sprites can then tell the SpriteCache to
load the frames and the sequences to resolve the
sprites.
PlayerColorRemap expected colors in linear space yet we provided them in gamma. We fix this by instead expecting gamma space colors and then converting them into linear space ourselves.
The cache in Locomotor that is populated via the UpdateCellBlocking method disagreed with the non-cached logic of IsBlockedBy when dealing with Mobile actors. The cache determined an actor to be moving if it was both movable and had horizontal movement types. IsBlockedBy determined an actor to be moving if it had horizontal movement types, but did not check if it was movable. This difference in checks could allow a mobile trait that was disabled or paused and which had horizontal movement to be treated differently be the two methods. UpdateCellBlocking would consider it not moving due to the disabled/paused trait. IsBlockedBy would consider it moving as it didn't care about the disabled/paused state of the trait.
Now, we unify the two methods to consider a mobile trait that is disabled/paused as not moving. This prevents HierarchicalPathFinder from crashing on the inconsistent state, i.e. when asked to path search through a cell of a mobile unit which has disabled or paused movement, but which has horizontal movement types from prior movement.
