Merge branch 'bleed' into rv-engine

.github/workflows/packaging.yml

@@ -67,7 +67,7 @@ jobs:
 
   macos:
     name: macOS Disk Image
-    runs-on: macos-11
+    runs-on: macos-13
     steps:
       - name: Clone Repository
         uses: actions/checkout@v4

OpenRA.Game/FileSystem/ZipFile.cs

@@ -114,6 +114,10 @@ public ReadWriteZipFile(string filename, bool create = false)
 				pkgStream.Position = 0;
 				pkg = new ZipFile(pkgStream);
 				Name = filename;
+
+				// Remove subfields that can break ZIP updating.
+				foreach (ZipEntry entry in pkg)
+					entry.ExtraData = null;
 			}
 
 			void Commit()

OpenRA.Game/Graphics/CursorManager.cs

@@ -91,10 +91,6 @@ public CursorManager(CursorProvider cursorProvider)
 			}
 
 			CreateOrUpdateHardwareCursors();
-
-			foreach (var s in sheetBuilder.AllSheets)
-				s.ReleaseBuffer();
-
 			Update();
 		}
 
@@ -128,6 +124,8 @@ void CreateOrUpdateHardwareCursors()
 				}
 			}
 
+			sheetBuilder.Current.ReleaseBuffer();
+
 			hardwareCursorsDoubled = graphicSettings.CursorDouble;
 		}
 

OpenRA.Game/Graphics/Sheet.cs

@@ -132,6 +132,7 @@ public void ReleaseBuffer()
 		{
 			if (!Buffered)
 				return;
+
 			dirty = true;
 			releaseBufferOnCommit = true;
 
@@ -140,6 +141,25 @@ public void ReleaseBuffer()
 				GetTexture();
 		}
 
+		public bool ReleaseBufferAndTryTransferTo(Sheet destination)
+		{
+			if (Size != destination.Size)
+				throw new ArgumentException("Destination sheet does not have the same size", nameof(destination));
+
+			var buffer = data;
+			ReleaseBuffer();
+
+			// Only transfer if the destination has no data that would be lost by overwriting.
+			if (buffer != null && destination.data == null && destination.texture == null)
+			{
+				Array.Clear(buffer, 0, buffer.Length);
+				destination.data = buffer;
+				return true;
+			}
+
+			return false;
+		}
+
 		public void Dispose()
 		{
 			texture?.Dispose();

OpenRA.Game/Graphics/SheetBuilder.cs

@@ -130,8 +130,13 @@ public Sprite Allocate(Size imageSize, float zRamp, in float3 spriteOffset, floa
 				var next = NextChannel(CurrentChannel);
 				if (next == null)
 				{
-					Current.ReleaseBuffer();
+					var previous = Current;
 					Current = allocateSheet();
+
+					// Reuse the backing buffer between sheets where possible.
+					// This avoids allocating additional buffers which the GC must clean up.
+					previous.ReleaseBufferAndTryTransferTo(Current);
+
 					sheets.Add(Current);
 					CurrentChannel = Type == SheetType.Indexed ? TextureChannel.Red : TextureChannel.RGBA;
 				}

OpenRA.Game/Graphics/SpriteCache.cs

@@ -80,9 +80,6 @@ public ISpriteFrame[] LoadFramesUncached(string filename)
 
 		public void LoadReservations(ModData modData)
 		{
-			foreach (var sb in SheetBuilders.Values)
-				sb.Current.CreateBuffer();
-
 			var pendingResolve = new List<(
 				string Filename,
 				int FrameIndex,

OpenRA.Launcher/Program.cs

@@ -20,25 +20,39 @@ static class Program
 		[STAThread]
 		static int Main(string[] args)
 		{
-			try
+			if (Debugger.IsAttached || args.Contains("--just-die"))
 			{
-				if (Debugger.IsAttached || args.Contains("--just-die"))
-					return (int)Game.InitializeAndRun(args);
-
-				AppDomain.CurrentDomain.UnhandledException += (_, e) => ExceptionHandler.HandleFatalError((Exception)e.ExceptionObject);
-
 				try
 				{
 					return (int)Game.InitializeAndRun(args);
 				}
-				catch (Exception e)
+				catch
 				{
-					ExceptionHandler.HandleFatalError(e);
-					return (int)RunStatus.Error;
+					// Flush logs before rethrowing, i.e. allowing the exception to go unhandled.
+					// try-finally won't work - an unhandled exception kills our process without running the finally block!
+					Log.Dispose();
+					throw;
 				}
+				finally
+				{
+					Log.Dispose();
+				}
+			}
+
+			AppDomain.CurrentDomain.UnhandledException += (_, e) => ExceptionHandler.HandleFatalError((Exception)e.ExceptionObject);
+
+			try
+			{
+				return (int)Game.InitializeAndRun(args);
+			}
+			catch (Exception e)
+			{
+				ExceptionHandler.HandleFatalError(e);
+				return (int)RunStatus.Error;
 			}
 			finally
 			{
+				// Flushing logs in finally block is okay here, as the catch block handles the exception.
 				Log.Dispose();
 			}
 		}

OpenRA.Mods.Common/Activities/Move/Move.cs

@@ -24,7 +24,7 @@ public class Move : Activity
 		public WAngle ActorFacingModifier { get; private set; }
 		readonly Mobile mobile;
 		readonly WDist nearEnough;
-		readonly Func<BlockedByActor, List<CPos>> getPath;
+		readonly Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> Path)> getPath;
 		readonly Actor ignoreActor;
 		readonly Color? targetLineColor;
 
@@ -39,6 +39,7 @@ public class Move : Activity
 		int carryoverProgress;
 		int lastMovePartCompletedTick;
 
+		bool alreadyAtDestination;
 		List<CPos> path;
 		CPos? destination;
 		int startTicks;
@@ -62,8 +63,11 @@ public Move(Actor self, CPos destination, Color? targetLineColor = null)
 
 			getPath = check =>
 			{
-				return mobile.PathFinder.FindPathToTargetCell(
-					self, new[] { mobile.ToCell }, destination, check, laneBias: false);
+				if (mobile.ToCell == destination)
+					return (true, PathFinder.NoPath);
+
+				return (false, mobile.PathFinder.FindPathToTargetCell(
+					self, new[] { mobile.ToCell }, destination, check, laneBias: false));
 			};
 
 			this.destination = destination;
@@ -82,10 +86,13 @@ public Move(Actor self, CPos destination, WDist nearEnough, Actor ignoreActor =
 			getPath = check =>
 			{
 				if (!this.destination.HasValue)
-					return PathFinder.NoPath;
+					return (false, PathFinder.NoPath);
+
+				if (mobile.ToCell == this.destination.Value)
+					return (true, PathFinder.NoPath);
 
-				return mobile.PathFinder.FindPathToTargetCell(
-					self, new[] { mobile.ToCell }, this.destination.Value, check, ignoreActor: ignoreActor);
+				return (false, mobile.PathFinder.FindPathToTargetCell(
+					self, new[] { mobile.ToCell }, this.destination.Value, check, ignoreActor: ignoreActor));
 			};
 
 			// Note: Will be recalculated from OnFirstRun if evaluateNearestMovableCell is true
@@ -97,7 +104,7 @@ public Move(Actor self, CPos destination, WDist nearEnough, Actor ignoreActor =
 			this.targetLineColor = targetLineColor;
 		}
 
-		public Move(Actor self, Func<BlockedByActor, List<CPos>> getPath, Color? targetLineColor = null)
+		public Move(Actor self, Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> Path)> getPath, Color? targetLineColor = null)
 		{
 			ActivityType = ActivityType.Move;
 
@@ -111,10 +118,11 @@ public Move(Actor self, Func<BlockedByActor, List<CPos>> getPath, Color? targetL
 			this.targetLineColor = targetLineColor;
 		}
 
-		List<CPos> EvalPath(BlockedByActor check)
+		(bool AlreadyAtDestination, List<CPos> Path) EvalPath(BlockedByActor check)
 		{
-			var path = getPath(check).TakeWhile(a => a != mobile.ToCell).ToList();
-			return path;
+			var (alreadyAtDestination, path) = getPath(check);
+			path = path.TakeWhile(a => a != mobile.ToCell).ToList();
+			return (alreadyAtDestination, path);
 		}
 
 		protected override void OnFirstRun(Actor self)
@@ -131,8 +139,8 @@ protected override void OnFirstRun(Actor self)
 			// TODO: Change this to BlockedByActor.Stationary after improving the local avoidance behaviour
 			foreach (var check in PathSearchOrder)
 			{
-				path = EvalPath(check);
-				if (path.Count > 0)
+				(alreadyAtDestination, path) = EvalPath(check);
+				if (alreadyAtDestination || path.Count > 0)
 					return;
 			}
 		}
@@ -152,6 +160,12 @@ public override bool Tick(Actor self)
 			if (mobile.IsTraitDisabled || mobile.IsTraitPaused)
 				return false;
 
+			if (alreadyAtDestination)
+			{
+				mobile.MoveResult = MoveResult.CompleteDestinationReached;
+				return true;
+			}
+
 			if (destination == mobile.ToCell)
 			{
 				if (hadNoPath)
@@ -235,7 +249,7 @@ public override bool Tick(Actor self)
 			// Something else might have moved us, so the path is no longer valid.
 			if (!Util.AreAdjacentCells(mobile.ToCell, nextCell))
 			{
-				path = EvalPath(BlockedByActor.Immovable);
+				(alreadyAtDestination, path) = EvalPath(BlockedByActor.Immovable);
 				return (null, false);
 			}
 
@@ -276,7 +290,7 @@ public override bool Tick(Actor self)
 				// There is no point in waiting for the other actor to move if it is incapable of moving.
 				if (!mobile.CanEnterCell(nextCell, ignoreActor, BlockedByActor.Immovable))
 				{
-					path = EvalPath(BlockedByActor.Immovable);
+					(alreadyAtDestination, path) = EvalPath(BlockedByActor.Immovable);
 					return (null, false);
 				}
 
@@ -302,7 +316,7 @@ public override bool Tick(Actor self)
 
 				// Calculate a new path
 				mobile.RemoveInfluence();
-				var newPath = EvalPath(BlockedByActor.All);
+				var (_, newPath) = EvalPath(BlockedByActor.All);
 				mobile.AddInfluence();
 
 				if (newPath.Count != 0)

OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs

@@ -117,7 +117,7 @@ public override bool Tick(Actor self)
 
 		protected int searchCellsTick = -1;
 
-		protected virtual List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
+		protected virtual (bool AlreadyAtDestination, List<CPos> Path) CalculatePathToTarget(Actor self, BlockedByActor check)
 		{
 			// PERF: Assume that candidate cells don't change within a tick to avoid repeated queries
 			// when Move enumerates different BlockedByActor values.
@@ -126,14 +126,21 @@ protected virtual List<CPos> CalculatePathToTarget(Actor self, BlockedByActor ch
 				SearchCells.Clear();
 				searchCellsTick = self.World.WorldTick;
 				foreach (var cell in Util.AdjacentCells(self.World, Target))
+				{
 					if (Mobile.CanStayInCell(cell) && Mobile.CanEnterCell(cell))
+					{
+						if (cell == self.Location)
+							return (true, PathFinder.NoPath);
+
 						SearchCells.Add(cell);
+					}
+				}
 			}
 
 			if (SearchCells.Count == 0)
-				return PathFinder.NoPath;
+				return (false, PathFinder.NoPath);
 
-			return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check);
+			return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check));
 		}
 
 		public override IEnumerable<Target> GetTargets(Actor self)

OpenRA.Mods.Common/Activities/Move/MoveOnto.cs

@@ -38,11 +38,14 @@ protected override bool ShouldStop(Actor self)
 			return Target.Type == TargetType.Terrain;
 		}
 
-		protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
+		protected override (bool AlreadyAtDestination, List<CPos> Path) CalculatePathToTarget(Actor self, BlockedByActor check)
 		{
+			if (lastVisibleTargetLocation == self.Location)
+				return (true, PathFinder.NoPath);
+
 			// If we are close to the target but can't enter, we wait.
 			if (!Mobile.CanEnterCell(lastVisibleTargetLocation) && Util.AreAdjacentCells(lastVisibleTargetLocation, self.Location))
-				return PathFinder.NoPath;
+				return (false, PathFinder.NoPath);
 
 			// PERF: Don't create a new list every run.
 			// PERF: Also reuse the already created list in the base class.
@@ -51,7 +54,7 @@ protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor c
 			else if (SearchCells[0] != lastVisibleTargetLocation)
 				SearchCells[0] = lastVisibleTargetLocation;
 
-			return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check);
+			return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check));
 		}
 	}
 }

OpenRA.Mods.Common/Activities/Move/MoveWithinRange.cs

@@ -48,8 +48,11 @@ protected override bool ShouldRepath(Actor self, CPos targetLocation)
 				|| !Mobile.CanInteractWithGroundLayer(self) || !Mobile.CanStayInCell(self.Location));
 		}
 
-		protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor check)
+		protected override (bool AlreadyAtDestination, List<CPos> Path) CalculatePathToTarget(Actor self, BlockedByActor check)
 		{
+			if (lastVisibleTargetLocation == self.Location)
+				return (true, PathFinder.NoPath);
+
 			// PERF: Assume that candidate cells don't change within a tick to avoid repeated queries
 			// when Move enumerates different BlockedByActor values.
 			if (searchCellsTick != self.World.WorldTick)
@@ -62,9 +65,9 @@ protected override List<CPos> CalculatePathToTarget(Actor self, BlockedByActor c
 			}
 
 			if (SearchCells.Count == 0)
-				return PathFinder.NoPath;
+				return (false, PathFinder.NoPath);
 
-			return Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check);
+			return (false, Mobile.PathFinder.FindPathToTargetCells(self, self.Location, SearchCells, check));
 		}
 
 		bool AtCorrectRange(WPos origin)

OpenRA.Mods.Common/Traits/Mobile.cs

@@ -804,7 +804,7 @@ public void EnteringCell(Actor self)
 			CrushAction(self, (notifyCrushed) => notifyCrushed.WarnCrush);
 		}
 
-		public Activity MoveTo(Func<BlockedByActor, List<CPos>> pathFunc) { return new Move(self, pathFunc); }
+		public Activity MoveTo(Func<BlockedByActor, (bool AlreadyAtDestination, List<CPos> Path)> pathFunc) { return new Move(self, pathFunc); }
 
 		Activity LocalMove(Actor self, WPos fromPos, WPos toPos, CPos cell)
 		{