Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cargo.EjectOnDeath subcell checks are completely broken #16353

Open
pchote opened this Issue Mar 25, 2019 · 0 comments

Comments

Projects
None yet
1 participant
@pchote
Copy link
Member

pchote commented Mar 25, 2019

While testing #16313 I noticed that the cargo ejection from the APCs in TS are broken - the 5 transported units will be ejected into 3 subcells, causing overlaps.

This is caused by three stacked problems:

  1. Subcell-availability checks are done immediately, but the passengers are added to the world in a FrameEndTask - and are therefore ignored.
  2. The Subcell-availability checks also ignore transient actors, so the passengers are still ignored even after fixing the first point.
  3. Locomotor.CanMoveFreelyInto fails to ignore ignoreActor if transientActors are enabled, so spawning is blocked by the transport after fixing the first two points.

The following diff seems to be enough to resolve the problem, but needs to be tested and refined before being PRed.

diff --git a/OpenRA.Mods.Common/Traits/Cargo.cs b/OpenRA.Mods.Common/Traits/Cargo.cs
index 29ab22e472..3b3ee0bb81 100644
--- a/OpenRA.Mods.Common/Traits/Cargo.cs
+++ b/OpenRA.Mods.Common/Traits/Cargo.cs
@@ -374,30 +374,35 @@ public void Load(Actor self, Actor a)
 
 		void INotifyKilled.Killed(Actor self, AttackInfo e)
 		{
-			if (Info.EjectOnDeath)
-				while (!IsEmpty(self) && CanUnload(true))
+			self.World.AddFrameEndTask(w =>
+			{
+				if (Info.EjectOnDeath)
 				{
-					var passenger = Unload(self);
-					var cp = self.CenterPosition;
-					var inAir = self.World.Map.DistanceAboveTerrain(cp).Length != 0;
-					var positionable = passenger.Trait<IPositionable>();
-					positionable.SetPosition(passenger, self.Location);
-
-					if (!inAir && positionable.CanEnterCell(self.Location, self, false))
+					while (!IsEmpty(self) && CanUnload(true))
 					{
-						self.World.AddFrameEndTask(w => w.Add(passenger));
-						var nbms = passenger.TraitsImplementing<INotifyBlockingMove>();
-						foreach (var nbm in nbms)
-							nbm.OnNotifyBlockingMove(passenger, passenger);
+						var passenger = Unload(self);
+						var cp = self.CenterPosition;
+						var inAir = self.World.Map.DistanceAboveTerrain(cp).Length != 0;
+						var positionable = passenger.Trait<IPositionable>();
+						positionable.SetPosition(passenger, self.Location);
+
+						if (!inAir && positionable.CanEnterCell(self.Location, self, true))
+						{
+							w.Add(passenger);
+							var nbms = passenger.TraitsImplementing<INotifyBlockingMove>();
+							foreach (var nbm in nbms)
+								nbm.OnNotifyBlockingMove(passenger, passenger);
+						}
+						else
+							passenger.Kill(e.Attacker);
 					}
-					else
-						passenger.Kill(e.Attacker);
 				}
 
-			foreach (var c in cargo)
-				c.Kill(e.Attacker);
+				foreach (var c in cargo)
+					c.Kill(e.Attacker);
 
-			cargo.Clear();
+				cargo.Clear();
+			});
 		}
 
 		void INotifyActorDisposing.Disposing(Actor self)
diff --git a/OpenRA.Mods.Common/Traits/World/Locomotor.cs b/OpenRA.Mods.Common/Traits/World/Locomotor.cs
index cf8234faf4..d4d373b426 100644
--- a/OpenRA.Mods.Common/Traits/World/Locomotor.cs
+++ b/OpenRA.Mods.Common/Traits/World/Locomotor.cs
@@ -241,7 +241,7 @@ public bool CanMoveFreelyInto(World world, Actor self, CPos cell, Actor ignoreAc
 			if (!check.HasCellCondition(CellConditions.TransientActors))
 				return true;
 
-			if (SharesCell && world.ActorMap.HasFreeSubCell(cell))
+			if (SharesCell && world.ActorMap.FreeSubCell(cell, SubCell.Any, a => a != ignoreActor) != SubCell.Invalid)
 				return true;
 
 			// PERF: Avoid LINQ.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.