Teach HierarchicalPathFinder about Immovable actors

[RoosterDragon]

By tracking updates on the ActorMap the HierarchicalPathFinder can be aware of actors moving around the map. We track a subset of immovable actors that always block. These actors can be treated as impassable obstacles just like terrain. When a path needs to be found the abstract path will guide the search around this subset of immovable actors just like it can guide the search around impassable terrain. For path searches that were previously imperformant because some immovable actors created a bottleneck that needed to be routed around, these will now be performant instead. Path searches with bottlenecks created by items such as trees, walls and buildings should see a performance improvement. Bottlenecks created by other units will not benefit.

We now maintain two sets of HPFs. One is aware of immovable actors and will be used for path searches that request BlockedByActor.Immovable, BlockedByActor.Stationary and BlockedByActor.All to guide that around the immovable obstacles. The other is aware of terrain only and will be used for searches that request BlockedByActor.None, or if an ignoreActor is provided. A new UI dropdown when using the /hpf command will allow switching between the visuals of the two sets.

---

We can turn on the /path-debug command to see the path search for the selected unit.

On bleed, when we try and path around terrain we can see HPF kicking in to guide the route. The yellow & white lines shows the explored cells of the path search. It "knows" to go around the terrain so we don't need to search many cells. Searching less cells means better performance.

If we repeat this same search but using actors to block the route, in this case just a wall, we see the search is less efficient. It didn't "know" about the wall in advance, so when it runs into the wall it has to expand the search by blobing outwards until it can find a way around. It searches more cells, which means less performance.

Using the /hpf command, we can see a visualisation of the HPF. On this PR, we can also select whether we see it for BlockedByActor.None or BlockedByActor.Immovable.

If we look at our wall example with the BlockedByActor.None setting, we can see it draws orange lines through the wall. It think there are routes between these points.

If we switch to the BlockedByActor.Immovable setting, we can see it has a new layout. It no longer draws lines between those points because it knows the wall is in the way.

Now if we try and find a path with our wall example, we see the improved behaviour, it "knows" about the wall and the search goes around it. We don't see the "blob" from when it didn't know.

Only some immovable actors are considered. Exceptions include anything that can sometimes be passed through:

• Temporary blockers, such as gates.
• Crushable actors. With /hpf we can select the heavytracked locomotor and we see the concrete wall becomes passable again. This is because heavytracked actors such as a Mammoth Tank can crush concrete walls.
• The bibs around buildings.

In general, I would expect blockages such as trees, buildings, uncrushable walls to benefit from the improved behaviour. Blockages from units, gates and crushable walls will continue to show poor performance.

Like the original PR #19591, the value proposition is much the same. We can significantly improve the performance of searching for paths with obstacles in the way. It costs us extra load time at the start of the game plus a small amount of ongoing overhead.

[PunkPun]

IMO this should block next release. Though it would definitelly be nice to have, like with all of the other PR's

[pchote]

SuperImmovableActor sounds a bit awkward IMO. How about StaticActor?

[PunkPun]

Perhaps ImmobileActor

[pchote]

The issue there is that there's potential for confusion with the Immobile trait.

[PunkPun]

But Immobile is most correct, as I understand this will include every actor that has IOccupiesSpace but doesn't have IMove (haven't looked at the code)

[RoosterDragon]

I knew there would be bikeshedding on this, so here was my line of thought and what I'd like to have and not have in the name:

BlockedByActor.Immovable defines what an immovable actor is. The set of actors we need to name is a subset of this. Conceptually it's like a level of BlockedByActor that sits between None and Immovable. This set of actors is every immovable actor except any actors that are crushable by the current locomotor, are temporary blockers, or are the transit only cells of a building.

Since we are defining a subset of an existing term, I'd quite like if immovable was in the name somewhere. Hence the thought of "super-immovable", or perhaps less fuzzy adjectives like "highly-immovable" or "permanently-immovable"could work.

Alternatively anything that also fulfils the "this name sounds like it would be a subset of 'immovable'" would satisfy me, even if immovable wasn't in the name. If we do use a distinct term, anything that could be confused with terminology elsewhere in the codebase should be avoided though.

[pchote]

Would AbstractPathBlockingActors, with a comment explaining that these are a subset of non-moving actors, be appropriate?

[PunkPun]

To me Immovable implies that the actor can't be moved by other actors, script or any ouside interance, but possibly can move by itself. Immobile implies that the unit can't move, period. I think names such as PermanentImmobleActors and immobileActors sound best. But I'm not aware of Immobile trait is and if this naming collision can be solved

[RoosterDragon]

Would AbstractPathBlockingActors, with a comment explaining that these are a subset of non-moving actors, be appropriate?

I could get behind the idea of using generic names such as cellsWithBlockingActors, ActorIsBlocking, ActorCellIsBlocking and then leaving the comments to explain the policy of what we're considering to be blocking.

[RoosterDragon]

Updated the naming in code, comments and commit messages to remove references to "super-immovable". Code uses the generic "blocking actors" naming as above. Comments refer to "the subset of immovable actors" when we need to be clear that only some of the actors are considered.

[PunkPun]

Force moved with a chrono tank

[PunkPun]

Landed aircraft update the HPF, I assume they shouldn't. Though landed + disabled aircraft should continue updating it

[RoosterDragon]

Force moved with a chrono tank

This visual is caused by the MoveAdjacentTo activity which searches for multiple locations. The activity provides multiple cells as it is happy as long as you can get to one of them, it doesn't mind about getting to a specific cell.

Although the multiple locations causes the visual to look busy, this is normal and expected behaviour.

You can also see a similar result by using the guard order. The unit that is guarding will use this same MoveAdjacentTo activity to get close to the unit that should be guarded.

Landed aircraft update the HPF, I assume they shouldn't. Though landed + disabled aircraft should continue updating it

I haven't added any special case for landed aircraft to the code. When landed it appears the aircraft meet all the conditions needed to be considered an immovable actor. This means the existing pathfinder would consider them to be an immovable actor that you would need to go around, and so HPF is acting correctly here by identifying the same thing and updating its graph. As a test case, try landing aircraft around an infantry on bleed. You can effectively construct a wall - the infantry unit will not be allowed to escape.

An actor is currently consider immovable if it lacks the Mobile trait. Aircraft do not have this trait. There is an interface named IPositionable which is implemented by Mobile, Aircraft, Husk, Crate, TDGunboat. The Locomotor class only cares about Mobile when deciding if something could be immovable or not. Perhaps there is an argument that this concept should be broadened to IPostionable things, but this would be a discussion that isn't specific to this PR. This would be a change to the current pathfinding logic as implemented by Locomotor. I also haven't thought this through in detail, I'm purely looking at the naming and thinking aloud.

[PunkPun]

I managed to crash the game. I've made infantry immovable. Add this code to e1

GrantConditionOnDeploy:
	DeployedCondition: deployed
	UndeployedCondition: undeployed
	UndeployOnMove: true
	UndeployOnPickup: true
Mobile:
	ImmovableCondition: !undeployed
	RequireForceMoveCondition: !undeployed

I made a long wall, sold one wall cell and deployed 1 infantry in it. I asked other infantry to move past the wall, with order being far enough to trigger HPF

Crash

Exception of type `System.InvalidOperationException`: Nullable object must have a value. at System.Nullable`1.get_Value() at OpenRA.Mods.Common.Pathfinder.HierarchicalPathFinder.<>c__DisplayClass39_0.b__0(CPos cell) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs:line 1028 at OpenRA.Mods.Common.Pathfinder.PathSearch.Expand() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Pathfinder/PathSearch.cs:line 237 at OpenRA.Mods.Common.Pathfinder.PathSearch.FindBidiPath(PathSearch first, PathSearch second) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Pathfinder/PathSearch.cs:line 327 at OpenRA.Mods.Common.Pathfinder.HierarchicalPathFinder.FindPath(Actor self, CPos source, CPos target, BlockedByActor check, Int32 heuristicWeightPercentage, Func`2 customCost, Actor ignoreActor, Boolean laneBias, PathFinderOverlay pathFinderOverlay) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs:line 850 at OpenRA.Mods.Common.Traits.PathFinder.FindPathToTargetCell(Actor self, IEnumerable`1 sources, CPos target, BlockedByActor check, Func`2 customCost, Actor ignoreActor, Boolean laneBias) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Traits/World/PathFinder.cs:line 89 at OpenRA.Mods.Common.Activities.Move.<>c__DisplayClass15_0.<.ctor>b__0(BlockedByActor check) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Activities/Move/Move.cs:line 78 at OpenRA.Mods.Common.Activities.Move.EvalPath(BlockedByActor check) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Activities/Move/Move.cs:line 108 at OpenRA.Mods.Common.Activities.Move.OnFirstRun(Actor self) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Mods.Common/Activities/Move/Move.cs:line 123 at OpenRA.Activities.Activity.TickOuter(Actor self) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Activities/Activity.cs:line 108 at OpenRA.Traits.ActivityUtils.RunActivity(Actor self, Activity act) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Traits/ActivityUtils.cs:line 31 at OpenRA.Actor.Tick() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Actor.cs:line 262 at OpenRA.World.Tick() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/World.cs:line 441 at OpenRA.Game.InnerLogicTick(OrderManager orderManager) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Game.cs:line 636 at OpenRA.Game.LogicTick() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Game.cs:line 651 at OpenRA.Game.Loop() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Game.cs:line 816 at OpenRA.Game.Run() in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Game.cs:line 869 at OpenRA.Game.InitializeAndRun(String[] args) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Game/Game.cs:line 308 at OpenRA.Launcher.Program.Main(String[] args) in /Users/gustas/Documents/GitHub/OpenRA/OpenRA.Launcher/Program.cs:line 32

[RoosterDragon]

I managed to crash the game. I've made infantry immovable.

Good catch. I've added the following code to ActorCellIsBlocking to handle this scenario.

var canShareCell = locomotor.Info.SharesCell && actorMap.HasFreeSubCell(cell);
if (canShareCell)
    return false;

The crash was being caused by the HPF which considered the deployed infantry to be blocking the cell. But this fails to consider units that can share cells like infantry. So the real pathfinder was happy to move past the deployed unit, because it didn't consider the cell to be blocked. The crash would occur when the real pathfinder tried to route through the cells, and the HPF would get confused and say "but that cell isn't reachable". By adding the new code above, we will correctly consider a cell with an immovable infantry it in to be passable. The actor might be immovable, but as long as there is free space in the cell, it can still be passed.

[PunkPun]

Now it doesn't crash but the checks still fail. If you place 5 infantry to block an entrance, but only make one of them in immobile, the hpf treats all of them as immovable and doesn't even think about nudging them

[RoosterDragon]

Same behaviour on bleed. If fact even without making one immovable as long as the cell is fully occupied with 5 troops, other infantry won't try and nudge them away but instead will go around. So not a shortcoming in HPF - it is correctly reflecting the behaviour of the local pathfinder.

[PunkPun]

In that case LGTM

[Mailaender]

It seems immovable actors that are pre-placed on the map are ignored:

while it properly updates the HPF when placing walls in-game.

[RoosterDragon]

It seems immovable actors that are pre-placed on the map are ignored

Fixed, thanks! I was doing

BuildImportantDataStructures()
AddActorsAlreadyOnMap()

Turns out this works much better :)

AddActorsAlreadyOnMap()
BuildImportantDataStructures()

[Mailaender]

Changelog

[RoosterDragon]

Note that #20329 weakens some of the benefits of this PR in relation to crushable actors.

Before this PR, we could tell for example in RA that a Concrete Wall could be crushed by Mammoth Tanks, which would therefore try to path through them. Other units knew they couldn't crush the wall and would path around them. The units that couldn't crush the wall would benefit from better performance by going around right away.

With #20329 in place we have to be more conservative. The Concrete Wall is crushable by some units, and now all units have to assume the wall is crushable and will a path search will attempt to go through them. For units which can't crush the wall, this leads to the same performance dip as described earlier where the path search must expand in a blob to determine how to go around the wall. This leads to less performance.

#20329 is a correctness fix, but maybe at some point down the road we can claw back the performance here.