From d2a36590781f8bb34984f101cab65d353c06d672 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 20 Aug 2022 16:21:40 +0100 Subject: [PATCH] Fix landed aircraft above ground level not removing influence on take off. 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. --- OpenRA.Mods.Common/Activities/Air/TakeOff.cs | 2 +- OpenRA.Mods.Common/Activities/DeliverUnit.cs | 2 -- OpenRA.Mods.Common/Traits/Air/Aircraft.cs | 15 +++++++++++---- OpenRA.Mods.Common/Traits/World/ActorMap.cs | 7 +++---- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/OpenRA.Mods.Common/Activities/Air/TakeOff.cs b/OpenRA.Mods.Common/Activities/Air/TakeOff.cs index deed92cfb8..2ed1d5a5ab 100644 --- a/OpenRA.Mods.Common/Activities/Air/TakeOff.cs +++ b/OpenRA.Mods.Common/Activities/Air/TakeOff.cs @@ -28,7 +28,7 @@ namespace OpenRA.Mods.Common.Activities if (aircraft.ForceLanding) return; - if (self.World.Map.DistanceAboveTerrain(aircraft.CenterPosition).Length >= aircraft.Info.MinAirborneAltitude) + if (!aircraft.HasInfluence()) return; // We are taking off, so remove influence in ground cells. diff --git a/OpenRA.Mods.Common/Activities/DeliverUnit.cs b/OpenRA.Mods.Common/Activities/DeliverUnit.cs index 8a67675c29..cf10bed5e6 100644 --- a/OpenRA.Mods.Common/Activities/DeliverUnit.cs +++ b/OpenRA.Mods.Common/Activities/DeliverUnit.cs @@ -86,8 +86,6 @@ namespace OpenRA.Mods.Common.Activities if (carryall.Carryable == null) return; - self.Trait().RemoveInfluence(); - var localOffset = carryall.CarryableOffset.Rotate(body.QuantizeOrientation(self.Orientation)); var targetPosition = self.CenterPosition + body.LocalToWorld(localOffset); var targetLocation = self.World.Map.CellContaining(targetPosition); diff --git a/OpenRA.Mods.Common/Traits/Air/Aircraft.cs b/OpenRA.Mods.Common/Traits/Air/Aircraft.cs index a27962f0d1..f2318dfecb 100644 --- a/OpenRA.Mods.Common/Traits/Air/Aircraft.cs +++ b/OpenRA.Mods.Common/Traits/Air/Aircraft.cs @@ -622,7 +622,7 @@ namespace OpenRA.Mods.Common.Traits public (CPos Cell, SubCell SubCell)[] OccupiedCells() { - if (!self.IsAtGroundLevel()) + if (self.World.Map.DistanceAboveTerrain(CenterPosition).Length >= Info.MinAirborneAltitude) return landingCells.Select(c => (c, SubCell.FullCell)).ToArray(); return new[] { (TopLeft, SubCell.FullCell) }; @@ -869,6 +869,10 @@ namespace OpenRA.Mods.Common.Traits public void AddInfluence(IEnumerable landingCells) { + if (this.landingCells.Any()) + throw new InvalidOperationException( + $"Cannot {nameof(AddInfluence)} until previous influence is removed with {nameof(RemoveInfluence)}"); + this.landingCells = landingCells; if (self.IsInWorld) self.World.ActorMap.AddInfluence(self, this); @@ -876,9 +880,7 @@ namespace OpenRA.Mods.Common.Traits public void AddInfluence(CPos landingCell) { - landingCells = new List { landingCell }; - if (self.IsInWorld) - self.World.ActorMap.AddInfluence(self, this); + AddInfluence(new[] { landingCell }); } public void RemoveInfluence() @@ -889,6 +891,11 @@ namespace OpenRA.Mods.Common.Traits landingCells = Enumerable.Empty(); } + public bool HasInfluence() + { + return landingCells.Any() || self.World.Map.DistanceAboveTerrain(CenterPosition).Length < Info.MinAirborneAltitude; + } + #endregion #region Implement IMove diff --git a/OpenRA.Mods.Common/Traits/World/ActorMap.cs b/OpenRA.Mods.Common/Traits/World/ActorMap.cs index e55bd3d432..b9b2c68d69 100644 --- a/OpenRA.Mods.Common/Traits/World/ActorMap.cs +++ b/OpenRA.Mods.Common/Traits/World/ActorMap.cs @@ -248,8 +248,7 @@ namespace OpenRA.Mods.Common.Traits { current = node.Actor; node = node.Next; - if (!current.Disposed) - return true; + return true; } return false; @@ -284,7 +283,7 @@ namespace OpenRA.Mods.Common.Traits var always = sub == SubCell.FullCell || sub == SubCell.Any; for (var i = layer[uv]; i != null; i = i.Next) - if (!i.Actor.Disposed && (i.SubCell == sub || i.SubCell == SubCell.FullCell || always)) + if (i.SubCell == sub || i.SubCell == SubCell.FullCell || always) yield return i.Actor; } @@ -386,7 +385,7 @@ namespace OpenRA.Mods.Common.Traits { var always = sub == SubCell.FullCell || sub == SubCell.Any; for (var i = layer[uv]; i != null; i = i.Next) - if ((always || i.SubCell == sub || i.SubCell == SubCell.FullCell) && !i.Actor.Disposed && withCondition(i.Actor)) + if ((always || i.SubCell == sub || i.SubCell == SubCell.FullCell) && withCondition(i.Actor)) return true; return false;