The restores the previous behaviour before FindUnitPathToTargetCell was introduced. This prevents callers such as the harvester code crashing when a harvester tries to route home to a refinery, but there are no refineries.
Some path searches, using PathSearch, were created directly at the callsite rather than using the pathfinder trait. This means some searches did not not benefit from the performance checks done in the pathfinder trait. It also means the pathfinder trait was not responsible for all pathing done in the game. Fix this with the following changes:
- Create a sensible shape for the IPathFinder interface and promote it to a trait interface, allowing theoretical replacements of the implementation. Ensure none of the concrete classes in OpenRA.Mods.Common.Pathfinder are exposed in the interface to ensure this is possible.
- Update the PathFinder class to implement the interface, and update several callsites manually running pathfinding code to instead call the IPathFinder interface.
- Overall, this allows any implementation of the IPathFinder interface to intercept and control all path searching performed by the game. Previously some searches would not have used it, and no alternate implementations were possible as the existing implementation was hardcoded into the interface shape.
Additionally:
- Move the responsibility of finding paths on completed path searches from pathfinder to path search, which is a more sensible location.
- Clean up the pathfinder pre-search optimizations.
The existing APIs surfaces for pathfinding are in a wonky shape. We rearrange various responsibilities to better locations and simplify some abstractions that aren't providing value.
- IPathSearch, BasePathSearch and PathSearch are combined into only PathSearch. Its role is now to run a search space over a graph, maintaining the open queue and evaluating the provided heuristic function. The builder-like methods (WithHeuristic, Reverse, FromPoint, etc) are removed in favour of optional parameters in static creation methods. This removes confusion between the builder-aspect and the search function itself. It also becomes responsible for applying the heuristic weight to the heuristic. This fixes an issue where an externally provided heuristic ignored the weighting adjustment, as previously the weight was baked into the default heuristic only.
- Reduce the IGraph interface to the concepts of nodes and edges. Make it non-generic as it is specifically for pathfinding, and rename to IPathGraph accordingly. This is sufficient for a PathSearch to perform a search over any given IGraph. The various customization options are concrete properties of PathGraph only.
- PathFinder does not need to deal with disposal of the search/graph, that is the caller's responsibility.
- Remove CustomBlock from PathGraph as it was unused.
- Remove FindUnitPathToRange as it was unused.
- Use PathFinder.NoPath as the single helper to represent no/empty paths.
The path cache was originally a moderate benefit, but over time a couple of things have conspired against it:
- Only paths with BlockedByActor.None are cached. Originally all paths regardless of blocking were cached but this was deemed unacceptable due to potentially returning outdated paths as actors move about. Paths with BlockedByActor.None are only invalidated if terrain conditions change, which are rarer.
- Move will try and find a path 4 times, trying with a different BlockedByActor check each time. BlockedByActor.None is the last check and only reached if the other searches fail. This is a rare scenario.
Overall, this means the hit rate for the cache is almost non-existent. Given the constraints on path validity it seems unlikely that the hit rate could be improved significantly, therefore it seems reasonable to remove the cache entirely to remove the overhead of cache management.
- Make Status the first field.
- Rename EstimatedTotal to EstimatedTotalCost to make it clearer it has the same unit as the CostSoFar field.
- Rename PreviousPos to PreviousNode as node terminology is a better match for usage.
- Rename CostForInvalidCell to PathCostForInvalidPath
- Add MovementCostForUnreachableCell
- Update usages of int.MaxValue and short.Maxvalue to use named constants where relevant.
- Update costs on ICustomMovementLayer to return short, for consistency with costs from Locomotor.
- Rename some methods to distinguish between path/movement cost.
Should save a trait look-up for each path search. Also ditch bogus null checks (this trait is a must-have anyway, so we *want* to crash if it's missing).
Add GrantConditionOn*Layer traits
This allows to
- drop some booleans from Locomotor
- drop a good part of the subterranean- and jumpjet-specific code/hacks from Mobile
- grant more than 1 condition per layer type (via multiple traits)
- easily add more traits of this kind for other layers
The previous implementation:
- Was failing to dispose of pooled layers.
- Was using a finalizer to allow undisposed layers to be reused.
This means all pooled layers are kept alive indefinitely until the map changes. If the finalizer is slow for any reason then the pathfiinder will allocate new layers when the pool runs out. Since these new layers are eventually stuffed back into the pool when the finalizer does run, this can theoretically leak unbounded memory until the pool goes out of scope. In practice it would leak tens of megabytes.
The new implementation ensures layers are disposed and pooled correctly to allow proper memory reuse. It also introduces some safeguards against memory leaks:
- A cap is set on the number of pooled layers. If more concurrent layers are needed than this, then the excess layers will not be pooled but instead be allowed to be garbage collected.
- No finalizer. An implementation that fails to call dispose simply allows the layer to be garbage collected instead.
The path caching works on the assumption that the time saved from reusing a cached path outweights the cost of caching it in the first place.
For unit paths, this assumption holds. For path searchs, we spend more time caching them then we save when we get to reuse these cached paths. This is because they are reused less often, and calculating their key is more expensive in comparison.
Introduced also a small Unit test project to prove it.
- Separated caching capabilities from PathFinder class to increase cohesion and maintainability.
Refactored the pathfinding algorithm by extracting methods based on responsibilities like
calculating costs and reordering functions. These changes should provide a in average a small increase in
pathfinding performance and maintainability.
- Optimized the pathfinder algorithm to reuse calculations like the
MovementCost and heuristics.
- Introduced base classes, IPathSearch and IPriorityQueue interfaces,
and restructured code to ease readability and testability
- Renamed the PathFinder related classes to more appropriate names. Made the
traits rely on the interface IPathfinder instead of concrete PathFinder
implementation.
- Massive performance improvements
- Solved error with harvesters' Heuristic
- Updated the heuristic to ease redability and adjustability. D can be
adjusted to offer best paths by decreasing and more performance by
increasing it
- Refactored the CellLayer<CellInfo> creation in its own Singleton class
- Extracted the graph abstraction onto an IGraph interface, making the
Pathfinder agnostic to the definition of world and terrain. This
abstraction can help in the future to be able to cache graphs for similar
classes and their costs, speeding up the pathfinder and being able to feed
the A* algorithm with different types of graphs like Hierarchical graphs
Automatically formatted all files via VS. This generally corrects indentation, removes trailing whitespace and corrects misplaced tabs or spaces. Manually tweaked a few files where required.
