From ad19506836804a7692df2ff4f45ea8d5995804c4 Mon Sep 17 00:00:00 2001 From: Ashley Newson Date: Tue, 20 May 2025 01:29:08 +0100 Subject: [PATCH] Fix TilingPath bugs affecting unofficial mods TilingPath had some bugs when working with single-point segments, which exist outside of the official mods. This change fixes a few problems: - Avoid unbounded length zero-cost, zero-progression loops that could be formed by an arbitrary number of single-point corners. These could pop up arbitrarily in the middle of paths. - Avoid zero-progression loops short-circuiting looped paths. - Fix missing end type matching during back-tracing stage. - Enhance lint check to ensure starts/ends have a valid directions. - Remove some dead code. --- OpenRA.Mods.Common/Lint/CheckMultiBrushes.cs | 7 ++++ OpenRA.Mods.Common/MapGenerator/MultiBrush.cs | 12 ++++++ OpenRA.Mods.Common/MapGenerator/TilingPath.cs | 37 +++++++++++-------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/OpenRA.Mods.Common/Lint/CheckMultiBrushes.cs b/OpenRA.Mods.Common/Lint/CheckMultiBrushes.cs index 850fb91873..7307d131e9 100644 --- a/OpenRA.Mods.Common/Lint/CheckMultiBrushes.cs +++ b/OpenRA.Mods.Common/Lint/CheckMultiBrushes.cs @@ -37,6 +37,13 @@ namespace OpenRA.Mods.Common.Lint foreach (var tile in multiBrush.PossibleTiles()) if (!templatedTerrainInfo.TryGetTerrainInfo(tile, out var _)) emitError($"Tileset {terrainInfoName} has invalid MultiBrush collection `{collectionName}`: tileset does not have tile {tile.Type},{tile.Index}"); + + if (multiBrush.Segment != null) + { + // Validate start and end have a direction. + _ = multiBrush.Segment.StartDirection; + _ = multiBrush.Segment.EndDirection; + } } catch (Exception e) when (e is ArgumentException || e is InvalidOperationException) { diff --git a/OpenRA.Mods.Common/MapGenerator/MultiBrush.cs b/OpenRA.Mods.Common/MapGenerator/MultiBrush.cs index 933cfeb933..574db23aeb 100644 --- a/OpenRA.Mods.Common/MapGenerator/MultiBrush.cs +++ b/OpenRA.Mods.Common/MapGenerator/MultiBrush.cs @@ -222,6 +222,18 @@ namespace OpenRA.Mods.Common.MapGenerator : (MatchesType(Start, matcher) || MatchesType(End, matcher)); public bool HasEndType(string matcher) => MatchesType(End, matcher); + + public static Direction TypeDirection(string type) + { + if (!Enum.TryParse(type.Split('.')[^1], out Direction direction)) + throw new InvalidOperationException("MultiBrushSegment has invalid direction"); + return direction; + } + + public Direction StartDirection + => TypeDirection(Start); + public Direction EndDirection + => TypeDirection(End); } /// A super template that can be used to paint both tiles and actors. diff --git a/OpenRA.Mods.Common/MapGenerator/TilingPath.cs b/OpenRA.Mods.Common/MapGenerator/TilingPath.cs index 85453d77be..a81f8b939d 100644 --- a/OpenRA.Mods.Common/MapGenerator/TilingPath.cs +++ b/OpenRA.Mods.Common/MapGenerator/TilingPath.cs @@ -249,9 +249,7 @@ namespace OpenRA.Mods.Common.MapGenerator public readonly CVec Offset; public readonly CVec Moves; public readonly CVec[] RelativePoints; - public readonly Direction[] Directions; - public readonly DirectionMask[] DirectionMasks; - public readonly DirectionMask[] ReverseDirectionMasks; + public readonly Direction EndDirection; public TilingSegment(MultiBrush multiBrush, int startId, int endId) { @@ -263,23 +261,13 @@ namespace OpenRA.Mods.Common.MapGenerator RelativePoints = multiBrush.Segment.Points .Select(p => p - multiBrush.Segment.Points[0]) .ToArray(); + EndDirection = multiBrush.Segment.EndDirection; - Directions = new Direction[RelativePoints.Length]; - DirectionMasks = new DirectionMask[RelativePoints.Length]; - ReverseDirectionMasks = new DirectionMask[RelativePoints.Length]; - - // Last point has no direction. - Directions[^1] = Direction.None; - DirectionMasks[^1] = 0; - ReverseDirectionMasks[^1] = 0; for (var i = 0; i < RelativePoints.Length - 1; i++) { var direction = DirectionExts.FromCVec(RelativePoints[i + 1] - RelativePoints[i]); if (direction == Direction.None) throw new ArgumentException("MultiBrushSegment has duplicate points in sequence"); - Directions[i] = direction; - DirectionMasks[i] = direction.ToMask(); - ReverseDirectionMasks[i] = direction.Reverse().ToMask(); } } } @@ -323,6 +311,7 @@ namespace OpenRA.Mods.Common.MapGenerator // order for a transition to be allowed at all, it must satisfy some constraints: // // - It must not regress backward along the path (but no immediate progress is OK). + // - If it makes exactly zero progress, it must end facing towards increasing progress. // - It must not deviate at any point in the segment beyond MaxDeviation from the path. // - It must not skip to much later path points (which may be within MaxDeviation). // @@ -688,6 +677,24 @@ namespace OpenRA.Mods.Common.MapGenerator return MaxCost; } + // If it's a zero-progress segment without deviation, only allow it if it directs + // towards a positive progression. + if (lowProgressionAcc == 0 && highProgressionAcc == 0) + { + var point = to; + var pointNext = to + segment.EndDirection.ToCVec(); + if (!deviations.ContainsXY(pointNext.X, pointNext.Y) || deviations[pointNext.X, pointNext.Y] == OverDeviation) + { + // Projected point escapes bounds or is in an excluded position. + return MaxCost; + } + + lowProgressionAcc = Progress(lowProgress[point.X, point.Y], lowProgress[pointNext.X, pointNext.Y]); + highProgressionAcc = Progress(highProgress[point.X, point.Y], highProgress[pointNext.X, pointNext.Y]); + if (lowProgressionAcc < 0 || highProgressionAcc < 0 || (lowProgressionAcc == 0 && highProgressionAcc == 0)) + return MaxCost; + } + // Satisfies all requirements. return deviationAcc; } @@ -896,7 +903,7 @@ namespace OpenRA.Mods.Common.MapGenerator (to, toTypeId) = TraceBackStep(to, toTypeId, true); // No need to check direction. If that is an issue, I have bigger problems to worry about. - while (to != pathStart) + while (to != pathStart || toTypeId != pathStartTypeId) (to, toTypeId) = TraceBackStep(to, toTypeId, false); }