Set an actor moving along several waypoints whilst the /path-debug command is active, then deselect the actor. Each waypoint will add more information to the debugging overlay until it crashes with "Maximum two records permitted." This resolves the crash by no longer adding new debugging information once the actor is deselected.
- Add prefixes to all message keys to provide context
- Use messages with attributes for some UI elements (dropdowns, dialogs, checkboxes, menus)
- Rename some class fields for consistency with translation keys
The Locomotor IsMoving check was allowing us to consider another actor that moving as not a blocker. However for some reason it also considered the actor trying to path being mobile as sufficient for this check to pass which did not make sense. We remove that extra check and inline the method.
This was a regression from 4a609bbee8 which changed the method from IsMovingInMyDirection (which required the lookup of the mobile trait) to just IsMoving. It should have removed the lookup as not required.
This fixes a crash in HPF which was considered the location as blocked when Locomotor considered it unblocked because the logic was not aligned. Removing this check aligns the logic and resolves the crash.
Using the local pathfinder, you could not find a path to an unreachable destination cell, but it was possible to find a path from an unreachable source cell if there was a reachable cells adjacent to it.
The hierarchical pathfinder did not have this behaviour and considering an unreachable source cell to block attempts to find a path.
Now, we unify the pathfinders to use a consistent behaviour, allowing paths from unreachable source cells to be found.
Occupied cells was defined by height yet we didn't update actor map on changing height. This in some scenarios could have caused the aircraft to forget to remove its influence from actor map
- Use LongBitSetAllocator and not BitSetAllocator. Using the wrong allocator means all string based checks and displays would provide incorrect results.
- Remove LongBitSetAllocator.Mask which wasn't being calculated or Reset correctly. We can use world.AllPlayersMask to provide the same effect at use sites.
When crushables and crates change their Location/TopLeft, their crushability is cached, but when their CenterPosition is changed, their cached crushability is not refreshed. Since their CrushableBy functions depends on IsAtGroundLevel, which depends on the CenterPosition, this means that when the crushability is cached it will depend on the current height of the object. If the height of the object changes, the cache is not refreshed and now contains out of date information.
The Locomotor cache and the HPF both cache this same information, but at different times. HPF caches immediately, but Locomotor caches on demand which means there can be a delay. This means they can have inconsistent, differing views of the crushability information. This eventually surfaces in a "The abstract path should never be searched for an unreachable point." error from HPF when it detects the inconsistency.
The bug is that Locomotor was caching information without refreshing it when required. Fixing this to refresh the cache when the CenterPosition changes is likely to have negative performance impacts. As would removing crushability from the cache. These would both be fixes that address the underlying bug.
The high impacts of a proper fix lead us to a workaround instead. If we set the CenterPosition before setting the Location, then when the Location is set and the caches are refreshed, the new CenterPosition is available when caching the crushability information. This means logic depending on IsAtGroundLevel will get the new information and cache a more up-to-date view of things. This means when changing both the CenterPosition and Location together we now cache correct information. However calls that set only the CenterPosition and not the Location can still result in a bad cache state. Although this is imperfect it is an improvement over current affairs, and has less impact.
When the Land activity is run, the aircraft adds influence to the cell so it cannot be used by other actors. When the TakeOff activity runs, it removes the influence so the cell can be used by other actors.
However, when a Carryall picks up a unit, it is told to Land with a vertical offset - it never reaches ground level. When the TakeOff activity runs, it saw the aircraft was above ground level and bailed out. The means the influence is never removed. The cell is now unusable despite the fact the Carryall has left.
To fix this, TakeOff now checks if influence was applied instead of checking if the aircraft is above ground level. If so, we know the Land activity had decided that influence was required, even if the aircraft has not made it to ground level. When TakeOff runs, it will treat it as a proper take off event even though the aircraft is already above ground level. This means influence will be removed and the cell will become accessible as intended.
In ActorMap, we also fix a design flaw where disposed actors where excluded from queries. This caused cache inconsistencies with clients using ActorMap.CellUpdated event to rely on updates. This event will not get called when the actor was disposed, so the downsteam client may have cached the actors at that location, only for them to "change" when the actor is later disposed. This could cause the Locomotor and HierarchicalPathFInder to have inconsistent views of the actors on the map, causing crashes if the inconsistent state broken some internal invariants. The only reason to exclude disposed actors would be to cover up for the actors not being removed properly from the map, which is fixed now aircraft are handled correctly. If ever an actor isn't removed from the actor map, then the caller needs fixing rather than having the actor map exclude it.
