When a move is made where the source and target locations are the same and no actual moving is required, a path of length 0 is returned. When a move cannot be made as there is no valid route, a path of length 0 is also returned. This means Move is unable to tell the difference between no movement required, and no path is possible. Currently it will hit the `hadNoPath` case and report CompleteDestinationBlocked.
To fix the scenario where the source and target location match, track a alreadyAtDestination field. When this scenario triggers, report CompleteDestinationReached instead.
This fixes activities that were using this result to inform their next actions. e.g. MoveOntoAndTurn would previously cancel the Turn portion of the activity, believing that the destination could not be reached. Now, it knows the destination was reached (since we are already there!) and will perform the turn.
Fixes a regression from 2c435c0506 - where the support actors must be able to path to the base actor in order to prevent them from spawning in isolated areas. If the base actor is immovable, they cannot path onto it because the base actor blocks them, so no support actors will spawn.
Fix this by allowing the support actors to path back to any cell adjacent to an immovable base actor, rather than requiring them to be able to path to its location directly.
When HarvesterBotModule is ordering idle harvesters to nearby resources, it previously scanned from the harvester's current location. Instead, it now scans from the location of the nearest refinery. As the harvester will likely make many runs between the resource and the refinery, it is better to choose a location near the refinery. This will minimise overall distance travelled to harvest the resource patch.
The AI uses HarvesterBotModule to check for idle harvesters, and give them harvest orders. By default it scans every 50 ticks (2 seconds at normal speed), and for any idle harvesters locates an ore patch and issues a harvest order. This FindNextResource to scan for a suitable ore path is quite expensive. If the AI has to scan for ore patches for several harvesters, then this can produce a noticeable lag spike. Additionally, when there are no available ore patches, the scan will just keep repeating since the harvesters will always be idle - thus the lag spikes repeat every 50 ticks.
To reduce the impact, there already exists a randomization on the first scan interval so that multiple different AIs scan on different ticks. By ensuring the AI players scan at different times, we avoid a huge lag spike where they all operate on the same tick.
To reduce the impact even more, we make four additional changes:
- Scans continue to be done every 50 ticks to detect harvesters. But we spread out the searches for ore patches over multiple later ticks. We'll only perform one ore patch search per tick. This means instead of ordering e.g. 30 harvesters on a single tick and creating a spike, we order one on each tick over the next 30 ticks instead. This spreads out the performance impact.
- When a harvester fails to locate any suitable ore patch, we put it on a longer cooldown, by default 5x the regular cooldown. We don't need to scan as often for these harvesters, since it'll take time for new resources to appear.
- We change the path search in FindNextResource from FindPathToTargetCellByPredicate to FindPathToTargetCells. The format in an undirected path search that must flood fill from the start location. If ore is on the other side of the map, this entails searching the whole map which is very expensive. By maintaining a lookup of resource types per cell, we can instead give the target locations directly to the path search. This lookup requires a small overhead to maintain, but allows for a far more efficient path search to be carried out. The search can be directed towards the target locations, and the hierarchical path finder can be employed resulting in a path search that explores far fewer cells. A few tweaks are made to ResourceClaimLayer to avoid it creating empty list entries when this can be avoided.
- We adjust how the enemy avoidance cost is done. Previously, this search used world.FindActorsInCircle to check for nearby enemies, but this check was done for every cell that was searched, and is itself quite expensive. Now, we create a series of "bins" and cache the additional cost for that bin. This is a less fine grained approach but is sufficient for our intended goal of "avoid resource patches with too many enemies nearby". The customCost function is now less expensive so we can reuse the avoidance cost stored for each bin, rather than calculating fresh for every cell.
When this hits the case that "As both ends are accessible, we can freely swap them." - we must note that we are reversing the path search and pass the information into the HierarchicalPathFinder. When a normal path search occurs, the actor trying to pathfind will never check its own location - and thus never gets blocked by itself. When a search is reversed, the search will check the actors location. If we inform the search it is doing done in reverse, it will special case this scenario and avoid the actor blocking itself. But if it is not told about this scenario, then this special case is not applied and no path will be found when in fact a path is possible.
When spawning starting units, or spawning units when collecting a crate, nearby locations will be used. If a nearby location cannot be reached, e.g. it is on top of a cliff or blocked in by trees, then any unit spawned there will be isolated which is not ideal.
Use the PathMightExistForLocomotorBlockedByImmovable method to filter nearby locations to those where the spawned unit can path back to the original location. This ensures the spawned unit is not isolated.
- AI places rally points at pathable locations. A pathable location isn't strictly required, but avoids the AI setting rally points at seemingly dumb locations. This is an addtional check on top of the existing buildability check.
- AI now evaluates rally points every AssignRallyPointsInterval (default 100 ticks). Invalid rally points aren't that harmful, so no need to check them every tick. Additionally we do a rolling update so rally points for multiple locations are spread across multiple ticks to reduce any potential lag spikes.
In 05ed9d9a73 we stopped caching the values with ToArray to resolve a desync. But even caching the enumerable can lead to a desync, so remove the caching entirely.
----
Let's explain how the code that cached values via ToArray could desync.
Usually, the cell given by `self.Location` matches with the cell given by `self.GetTargetablePositions()`. However if the unit is moving and close to the boundary between two cells, it is possible for the targetable position to be an adjacent cell instead.
Combined with the fact hovering over the unit will evaluate `CurrentAdjacentCells` only for the local player and not everybody, the following sequence becomes possible to induce a desync:
- As the APC is moving into the last cell before unloading, the local player hovers over it. `self.Location` is the last cell, but `self.GetTargetablePositions()` gives the *previous* cell (as the unit is close to the boundary between the cells)
- The local player then caches `CurrentAdjacentCells`. The cache key of `self.Location` is the final cell, but the values are calculated for `self.GetTargetablePositions()` of an *adjacent* cell.
- When the order to unload is resolved, the cache key of `CurrentAdjacentCells` is already `self.Location` and so `CurrentAdjacentCells` is *not* updated.
- The units unload into cells based on the *adjacent* cell.
Then, for other players in the game:
- The hover does nothing for these players.
- When the order is resolved, `CurrentAdjacentCells` is out of date and is re-evaluated.
- `self.Location` and `self.GetTargetablePositions()` are both the last cell, because the unit has finished moving.
- So the cache is updated with a key of `self.Location` and values from the *same* cell.
- The units unload into cells based on the *current* cell.
As the units unload into different cells, a desync occurs. Ultimately the cause here is that cache key is insufficient - `self.Location` can have the same value but the output can differ. The function isn't a pure function so memoizing the result via `ToArray()` isn't sound.
Reverting it to cache the enumerable, which is then lazily re-evaluated reduces the scope of possible desyncs but is NOT a full solve. The cached enumerable caches the result of `Actor.GetTargetablePositions()` which isn't a fully lazy sequence. A different result is returned depending on `EnabledTargetablePositions.Any()`. Therefore, if the traits were to enable/disable inbetween, then we can still end up with different results. Memoizing the enumerable isn't sound either!
Currently our only trait is `HitShape` which is enabled based on conditions. A condition that enables/disables it based on movement would be one way to trigger this scenario. Let's say you have a unit where you toggle between two hit shapes when it is moving and when it stops moving. That would allow you to replicate the above scenario once again.
Instead of trying to come up with a sound caching mechanism in the face of a series of complex inputs, we just give up on trying to cache this information at all.
The AI would often invoke this method inside of loops, searching for a different category of queue each time. This would result in multiple searches against the trait dictionary to locate matching queues. Now we alter the method to create a lookup of all the queues keyed by category. This allows a single trait search to be performed.
UnitBuilderBotModule and BaseBuilderBotModule are updated to fetch this lookup once when required, and pass the results along to avoid calling the method more times than necessary. This improves their performance.
Updating squads is the most expensive part of SquadManagerBotModule. It involves ticking the current squad state. This usually involves finding nearby enemies and evaluating the fuzzy state machine to decide whether to interact with those enemies. Since all the AI squads for a player get ordered on the same tick, this can result in a lag spike.
To reduce the impact, we'll spread out the updates over multiple ticks. This means overall all the AI squads will still be refreshed every interval, but it'll be a rolling update rather than all at once. By spreading out the updates we avoid a lag spike from the cumulative updates of all the squads. Now the lag spike is reduced to the worst any single squad update can incur.
The StateMachine offered a feature to remember the previous state and allow reverting to it. However this feature is unused. Remove it to allow the previous states to be reclaimed by the GC earlier.
If a long path was being visualized with the path-debug command it would generate renderables for everything on the path, even for parts of the path that would be offscreen. Add some simplistic culling so the performance impact is reduced.
Several activities that queue child Move activities can get into a bad scenario where the actor is pathfinding and then gets stuck because the destination is unreachable. When the Move activity then completes, then parent activity sees it has yet to reach the destination and tries to move again. However, the actor is still blocked in the same spot as before and thus the movment finishes immediately. This causes a performance death spiral where the actor attempts to pathfind every tick. The pathfinding attempt can also be very expensive if it must exhaustively check the whole map to determine no route is possible.
In order to prevent blocked actors from running into this scenario, we introduce MoveCooldownHelper. In its default setup it allows the parent activity to bail out if the actor was blocked during a pathfinding attempt. This means the activity will be dropped rather than trying to move endlessly. It also has an option to allow retrying if pathfinding was blocked, but applies a cooldown to avoid the performance penalty. For activities such as Enter, this means the actors will still try and enter their target if it is unreachable, but will only attempt once a second now rather than every tick.
MoveAdjacentTo will now cancel if it fails to reach the destination. This fixes MoveOntoAndTurn to skip the Turn if the move didn't reach the intended destination. Any other derived classes will similarly benefit from skipping follow-up actions.
- EditorActorLayer now tracks previews on map with a SpatiallyPartitioned instead of a Dictionary. This allows the copy-paste logic to call an efficient PreviewsInCellRegion method, instead of asking for previews cell-by-cell.
- EditorActorPreview subscribes to the CellEntryChanged methods on the map. Previously the preview was refreshed regardless of which cell changed. Now the preview only regenerates if the preview's footprint has been affected.
The SupportPowerManager and WithSpriteBody trait captured the ActorInitializer in lambda expressions, which keeps it alive as long as the trait. The lambdas didn't need to capture the ActorInitializer, so rejig them to allow the ActorInitializer to be reclaimed after the traits have been created. As the TypeDictionary in the ActorInitializer can be quite large, this helps reduce memory usage.
When dealing with isometric maps, the copy paste region needs to copy the CellCoords area covered by the CellRegion. This is equivalent to the selected rectangle on screen. Using the cell region itself invokes cell->map->cell conversion that doesn't roundtrip and thus some of the selected cells don't get copied.
Also when pasting terrain + actors, we need to fix the sequencing to clear actors on the current terrain, before adjusting the height and pasting in new actors. The current sequencing means we are clearing actors after having adjusted the terrain height, and affects the wrong area.
