From fa87befeff06d869529504a570f55d0dcc110d05 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Wed, 19 Aug 2015 23:47:18 +0100 Subject: [PATCH 01/10] Add missing CellEntryChanged checks. --- OpenRA.Game/Map/Map.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/OpenRA.Game/Map/Map.cs b/OpenRA.Game/Map/Map.cs index a0a0fd5fb0..ef15b56ceb 100644 --- a/OpenRA.Game/Map/Map.cs +++ b/OpenRA.Game/Map/Map.cs @@ -292,6 +292,8 @@ namespace OpenRA { var ret = new CellLayer(tileShape, size); ret.Clear(tileRef); + if (MaximumTerrainHeight > 0) + ret.CellEntryChanged += UpdateProjection; return ret; }); @@ -299,6 +301,8 @@ namespace OpenRA { var ret = new CellLayer(tileShape, size); ret.Clear(0); + if (MaximumTerrainHeight > 0) + ret.CellEntryChanged += UpdateProjection; return ret; }); From 4eacb6e5c95859f00ba53d41307a424ed8e3b4dd Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Wed, 19 Aug 2015 23:48:07 +0100 Subject: [PATCH 02/10] Transparently cache results of GetTerrainIndex in Map. This method performs an expensive calculation and is called often during pathfinding. We create a cache of the terrain indicies for the map to vastly reduce the cost. --- OpenRA.Game/Map/Map.cs | 28 ++++++++++++++++++++++++++-- OpenRA.Game/Map/TileSet.cs | 6 +++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/OpenRA.Game/Map/Map.cs b/OpenRA.Game/Map/Map.cs index ef15b56ceb..2f578880cd 100644 --- a/OpenRA.Game/Map/Map.cs +++ b/OpenRA.Game/Map/Map.cs @@ -246,6 +246,7 @@ namespace OpenRA [FieldLoader.Ignore] public Lazy> MapHeight; [FieldLoader.Ignore] public CellLayer CustomTerrain; + [FieldLoader.Ignore] CellLayer cachedTerrainIndexes; [FieldLoader.Ignore] bool initializedCellProjection; [FieldLoader.Ignore] CellLayer cellProjection; @@ -951,9 +952,32 @@ namespace OpenRA public byte GetTerrainIndex(CPos cell) { + const short InvalidCachedTerrainIndex = -1; + + // Lazily initialize a cache for terrain indexes. + if (cachedTerrainIndexes == null) + { + cachedTerrainIndexes = new CellLayer(this); + cachedTerrainIndexes.Clear(InvalidCachedTerrainIndex); + + // Invalidate the entry for a cell if anything could cause the terrain index to change. + Action invalidateTerrainIndex = c => cachedTerrainIndexes[c] = InvalidCachedTerrainIndex; + CustomTerrain.CellEntryChanged += invalidateTerrainIndex; + MapTiles.Value.CellEntryChanged += invalidateTerrainIndex; + } + var uv = cell.ToMPos(this); - var custom = CustomTerrain[uv]; - return custom != byte.MaxValue ? custom : cachedTileSet.Value.GetTerrainIndex(MapTiles.Value[uv]); + var terrainIndex = cachedTerrainIndexes[uv]; + + // Cache terrain indexes per cell on demand. + if (terrainIndex == InvalidCachedTerrainIndex) + { + var custom = CustomTerrain[uv]; + terrainIndex = cachedTerrainIndexes[uv] = + custom != byte.MaxValue ? custom : cachedTileSet.Value.GetTerrainIndex(MapTiles.Value[uv]); + } + + return (byte)terrainIndex; } public TerrainTypeInfo GetTerrainInfo(CPos cell) diff --git a/OpenRA.Game/Map/TileSet.cs b/OpenRA.Game/Map/TileSet.cs index 34896f3d36..d60075d369 100644 --- a/OpenRA.Game/Map/TileSet.cs +++ b/OpenRA.Game/Map/TileSet.cs @@ -74,7 +74,7 @@ namespace OpenRA public readonly bool PickAny; public readonly string Category; - TerrainTileInfo[] tileInfo; + readonly TerrainTileInfo[] tileInfo; public TerrainTemplateInfo(ushort id, string[] images, int2 size, byte[] tiles) { @@ -177,7 +177,7 @@ namespace OpenRA public readonly bool IgnoreTileSpriteOffsets; [FieldLoader.Ignore] - public readonly Dictionary Templates = new Dictionary(); + public readonly IReadOnlyDictionary Templates; [FieldLoader.Ignore] public readonly TerrainTypeInfo[] TerrainInfo; @@ -217,7 +217,7 @@ namespace OpenRA // Templates Templates = yaml["Templates"].ToDictionary().Values - .Select(y => new TerrainTemplateInfo(this, y)).ToDictionary(t => t.Id); + .Select(y => new TerrainTemplateInfo(this, y)).ToDictionary(t => t.Id).AsReadOnly(); } public TileSet(string name, string id, string palette, TerrainTypeInfo[] terrainInfo) From ac1658c9ce558ac25626e1ad0dc60a81d7a97973 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Wed, 19 Aug 2015 23:50:36 +0100 Subject: [PATCH 03/10] Refactor movement cost method to avoid repeated terrain information lookups. As the world tileset is fixed, the pathfinder can look up the terrain information for that tileset on creation. This is implemented by the WorldMovementInfo struct. When calculating node costs, this allows the pathfinder to avoid having to repeat this expensive dictionary lookup on every node. --- OpenRA.Mods.Common/Pathfinder/PathGraph.cs | 14 +++------ OpenRA.Mods.Common/Traits/Mobile.cs | 35 +++++++++++++++++----- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs index 4336184890..6d643161d8 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs @@ -71,6 +71,7 @@ namespace OpenRA.Mods.Common.Pathfinder readonly CellConditions checkConditions; readonly MobileInfo mobileInfo; + readonly MobileInfo.WorldMovementInfo worldMovementInfo; CellLayer cellInfo; public PathGraph(CellLayer cellInfo, MobileInfo mobileInfo, Actor actor, World world, bool checkForBlocked) @@ -78,6 +79,7 @@ namespace OpenRA.Mods.Common.Pathfinder this.cellInfo = cellInfo; World = world; this.mobileInfo = mobileInfo; + worldMovementInfo = mobileInfo.GetWorldMovementInfo(world); Actor = actor; LaneBias = 1; checkConditions = checkForBlocked ? CellConditions.TransientActors : CellConditions.None; @@ -123,17 +125,9 @@ namespace OpenRA.Mods.Common.Pathfinder int GetCostToNode(CPos destNode, CVec direction) { - int movementCost; - if (mobileInfo.CanEnterCell( - World, - Actor, - destNode, - out movementCost, - IgnoredActor, - checkConditions) && !(CustomBlock != null && CustomBlock(destNode))) - { + var movementCost = mobileInfo.MovementCostToEnterCell(worldMovementInfo, Actor, destNode, IgnoredActor, checkConditions); + if (movementCost != int.MaxValue && !(CustomBlock != null && CustomBlock(destNode))) return CalculateCellCost(destNode, direction, movementCost); - } return Constants.InvalidNode; } diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index 91351f3697..b3249393db 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -125,6 +125,17 @@ namespace OpenRA.Mods.Common.Traits } } + public struct WorldMovementInfo + { + internal readonly World World; + internal readonly TerrainInfo[] TerrainInfos; + internal WorldMovementInfo(World world, MobileInfo info) + { + World = world; + TerrainInfos = info.TilesetTerrainInfo[world.TileSet]; + } + } + public readonly Cache TilesetTerrainInfo; public readonly Cache TilesetMovementClass; @@ -136,14 +147,19 @@ namespace OpenRA.Mods.Common.Traits public int MovementCostForCell(World world, CPos cell) { - if (!world.Map.Contains(cell)) + return MovementCostForCell(world.Map, TilesetTerrainInfo[world.TileSet], cell); + } + + int MovementCostForCell(Map map, TerrainInfo[] terrainInfos, CPos cell) + { + if (!map.Contains(cell)) return int.MaxValue; - var index = world.Map.GetTerrainIndex(cell); + var index = map.GetTerrainIndex(cell); if (index == byte.MaxValue) return int.MaxValue; - return TilesetTerrainInfo[world.TileSet][index].Cost; + return terrainInfos[index].Cost; } public int CalculateTilesetMovementClass(TileSet tileset) @@ -236,12 +252,17 @@ namespace OpenRA.Mods.Common.Traits return false; } - public bool CanEnterCell(World world, Actor self, CPos cell, out int movementCost, Actor ignoreActor = null, CellConditions check = CellConditions.All) + public WorldMovementInfo GetWorldMovementInfo(World world) { - if ((movementCost = MovementCostForCell(world, cell)) == int.MaxValue) - return false; + return new WorldMovementInfo(world, this); + } - return CanMoveFreelyInto(world, self, cell, ignoreActor, check); + public int MovementCostToEnterCell(WorldMovementInfo worldMovementInfo, Actor self, CPos cell, Actor ignoreActor = null, CellConditions check = CellConditions.All) + { + var cost = MovementCostForCell(worldMovementInfo.World.Map, worldMovementInfo.TerrainInfos, cell); + if (cost == int.MaxValue || !CanMoveFreelyInto(worldMovementInfo.World, self, cell, ignoreActor, check)) + return int.MaxValue; + return cost; } public SubCell GetAvailableSubCell( From 18478646d469d94a2fcf0c74a491836ae411a977 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 19:14:46 +0100 Subject: [PATCH 04/10] Avoid multiple ToMPos calls in ActorMap. --- OpenRA.Game/Traits/World/ActorMap.cs | 37 +++++++++++++++++----------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/OpenRA.Game/Traits/World/ActorMap.cs b/OpenRA.Game/Traits/World/ActorMap.cs index 4c26c24408..b65d487ac6 100644 --- a/OpenRA.Game/Traits/World/ActorMap.cs +++ b/OpenRA.Game/Traits/World/ActorMap.cs @@ -188,20 +188,22 @@ namespace OpenRA.Traits public IEnumerable GetUnitsAt(CPos a) { - if (!influence.Contains(a)) + var uv = a.ToMPos(map); + if (!influence.Contains(uv)) yield break; - for (var i = influence[a]; i != null; i = i.Next) + for (var i = influence[uv]; i != null; i = i.Next) if (!i.Actor.Disposed) yield return i.Actor; } public IEnumerable GetUnitsAt(CPos a, SubCell sub) { - if (!influence.Contains(a)) + var uv = a.ToMPos(map); + if (!influence.Contains(uv)) yield break; - for (var i = influence[a]; i != null; i = i.Next) + for (var i = influence[uv]; i != null; i = i.Next) if (!i.Actor.Disposed && (i.SubCell == sub || i.SubCell == SubCell.FullCell)) yield return i.Actor; } @@ -243,20 +245,22 @@ namespace OpenRA.Traits // NOTE: always includes transients with influence public bool AnyUnitsAt(CPos a) { - if (!influence.Contains(a)) + var uv = a.ToMPos(map); + if (!influence.Contains(uv)) return false; - return influence[a] != null; + return influence[uv] != null; } // NOTE: can not check aircraft public bool AnyUnitsAt(CPos a, SubCell sub, bool checkTransient = true) { - if (!influence.Contains(a)) + var uv = a.ToMPos(map); + if (!influence.Contains(uv)) return false; var always = sub == SubCell.FullCell || sub == SubCell.Any; - for (var i = influence[a]; i != null; i = i.Next) + for (var i = influence[uv]; i != null; i = i.Next) { if (always || i.SubCell == sub || i.SubCell == SubCell.FullCell) { @@ -275,11 +279,12 @@ namespace OpenRA.Traits // NOTE: can not check aircraft public bool AnyUnitsAt(CPos a, SubCell sub, Func withCondition) { - if (!influence.Contains(a)) + var uv = a.ToMPos(map); + if (!influence.Contains(uv)) return false; var always = sub == SubCell.FullCell || sub == SubCell.Any; - for (var i = influence[a]; i != null; i = i.Next) + for (var i = influence[uv]; i != null; i = i.Next) if ((always || i.SubCell == sub || i.SubCell == SubCell.FullCell) && !i.Actor.Disposed && withCondition(i.Actor)) return true; @@ -290,10 +295,11 @@ namespace OpenRA.Traits { foreach (var c in ios.OccupiedCells()) { - if (!influence.Contains(c.First)) + var uv = c.First.ToMPos(map); + if (!influence.Contains(uv)) continue; - influence[c.First] = new InfluenceNode { Next = influence[c.First], SubCell = c.Second, Actor = self }; + influence[uv] = new InfluenceNode { Next = influence[uv], SubCell = c.Second, Actor = self }; List triggers; if (cellTriggerInfluence.TryGetValue(c.First, out triggers)) @@ -306,12 +312,13 @@ namespace OpenRA.Traits { foreach (var c in ios.OccupiedCells()) { - if (!influence.Contains(c.First)) + var uv = c.First.ToMPos(map); + if (!influence.Contains(uv)) continue; - var temp = influence[c.First]; + var temp = influence[uv]; RemoveInfluenceInner(ref temp, self); - influence[c.First] = temp; + influence[uv] = temp; List triggers; if (cellTriggerInfluence.TryGetValue(c.First, out triggers)) From dab53f403d82451396613233765edbe378c13f35 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 19:31:29 +0100 Subject: [PATCH 05/10] Provide a hand-written enumerator for ActorMap.GetUnitsAt(CPos). --- OpenRA.Game/Traits/World/ActorMap.cs | 38 ++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/OpenRA.Game/Traits/World/ActorMap.cs b/OpenRA.Game/Traits/World/ActorMap.cs index b65d487ac6..82a99ccd98 100644 --- a/OpenRA.Game/Traits/World/ActorMap.cs +++ b/OpenRA.Game/Traits/World/ActorMap.cs @@ -9,6 +9,7 @@ #endregion using System; +using System.Collections; using System.Collections.Generic; using System.Drawing; using System.Linq; @@ -186,15 +187,42 @@ namespace OpenRA.Traits actorShouldBeRemoved = removeActorPosition.Contains; } + sealed class UnitsAtEnumerator : IEnumerator + { + InfluenceNode node; + public UnitsAtEnumerator(InfluenceNode node) { this.node = node; } + public void Reset() { throw new NotSupportedException(); } + public Actor Current { get; private set; } + object IEnumerator.Current { get { return Current; } } + public void Dispose() { } + public bool MoveNext() + { + while (node != null) + { + Current = node.Actor; + node = node.Next; + if (!Current.Disposed) + return true; + } + + return false; + } + } + + sealed class UnitsAtEnumerable : IEnumerable + { + readonly InfluenceNode node; + public UnitsAtEnumerable(InfluenceNode node) { this.node = node; } + public IEnumerator GetEnumerator() { return new UnitsAtEnumerator(node); } + IEnumerator IEnumerable.GetEnumerator() { return GetEnumerator(); } + } + public IEnumerable GetUnitsAt(CPos a) { var uv = a.ToMPos(map); if (!influence.Contains(uv)) - yield break; - - for (var i = influence[uv]; i != null; i = i.Next) - if (!i.Actor.Disposed) - yield return i.Actor; + return Enumerable.Empty(); + return new UnitsAtEnumerable(influence[uv]); } public IEnumerable GetUnitsAt(CPos a, SubCell sub) From be59d045cea90a53b907fd00b3ef51cfd94e1115 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 19:37:35 +0100 Subject: [PATCH 06/10] Only enumerate crushable traits once in MobileInfo.IsBlockedBy. --- OpenRA.Mods.Common/Traits/Mobile.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index b3249393db..444cfac766 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -242,11 +242,17 @@ namespace OpenRA.Mods.Common.Traits // If the other actor in our way cannot be crushed, we are blocked. var crushables = otherActor.TraitsImplementing(); - if (!crushables.Any()) - return true; + var lacksCrushability = true; foreach (var crushable in crushables) + { + lacksCrushability = false; if (!crushable.CrushableBy(Crushes, self.Owner)) return true; + } + + // If there are no crushable traits at all, this means the other actor cannot be crushed - we are blocked. + if (lacksCrushability) + return true; // We are not blocked by the other actor. return false; From 7d44eb953e0ad8630187cf410913c44d541a8c79 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 22:38:14 +0100 Subject: [PATCH 07/10] Reduce size of GraphConnection for allocation efficiency. --- OpenRA.Mods.Common/Pathfinder/PathGraph.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs index 6d643161d8..603a8f8cd1 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs @@ -47,13 +47,11 @@ namespace OpenRA.Mods.Common.Pathfinder public struct GraphConnection { - public readonly CPos Source; public readonly CPos Destination; public readonly int Cost; - public GraphConnection(CPos source, CPos destination, int cost) + public GraphConnection(CPos destination, int cost) { - Source = source; Destination = destination; Cost = cost; } @@ -117,7 +115,7 @@ namespace OpenRA.Mods.Common.Pathfinder var neighbor = position + directions[i]; var movementCost = GetCostToNode(neighbor, directions[i]); if (movementCost != Constants.InvalidNode) - validNeighbors.AddLast(new GraphConnection(position, neighbor, movementCost)); + validNeighbors.AddLast(new GraphConnection(neighbor, movementCost)); } return validNeighbors; From 76303e96992c28dc599aa2f4ecd47c0746c2544d Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 20:35:14 +0100 Subject: [PATCH 08/10] In PathGraph.GetConnections, return a List of neighbors directly, rather than a LinkedList typed an IEnumerable. The caller can enumerate the list more efficiently without the IEnumerable indirection, and the reduced memory allocation is marginally faster than allocating a linked list and several nodes. --- OpenRA.Mods.Common/Pathfinder/PathGraph.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs index 603a8f8cd1..3204b5282e 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs @@ -23,7 +23,7 @@ namespace OpenRA.Mods.Common.Pathfinder /// /// Gets all the Connections for a given node in the graph /// - IEnumerable GetConnections(CPos position); + List GetConnections(CPos position); /// /// Retrieves an object given a node in the graph @@ -100,7 +100,7 @@ namespace OpenRA.Mods.Common.Pathfinder new[] { new CVec(1, -1), new CVec(1, 0), new CVec(-1, 1), new CVec(0, 1), new CVec(1, 1) }, }; - public IEnumerable GetConnections(CPos position) + public List GetConnections(CPos position) { var previousPos = cellInfo[position].PreviousPos; @@ -108,14 +108,14 @@ namespace OpenRA.Mods.Common.Pathfinder var dy = position.Y - previousPos.Y; var index = dy * 3 + dx + 4; - var validNeighbors = new LinkedList(); var directions = DirectedNeighbors[index]; + var validNeighbors = new List(directions.Length); for (var i = 0; i < directions.Length; i++) { var neighbor = position + directions[i]; var movementCost = GetCostToNode(neighbor, directions[i]); if (movementCost != Constants.InvalidNode) - validNeighbors.AddLast(new GraphConnection(neighbor, movementCost)); + validNeighbors.Add(new GraphConnection(neighbor, movementCost)); } return validNeighbors; From d9dd96ca35697c634253bce38f39d4dcbefd5caa Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 20 Aug 2015 21:12:21 +0100 Subject: [PATCH 09/10] Speed up Map.ContainsAllProjectedCellsCovering on flat maps. This method gets called often via Contains calls. We can significantly speed up the method for flat maps since we know the projection and it is trivial to perform. This avoids an expensive projection lookup. --- OpenRA.Game/Map/Map.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/OpenRA.Game/Map/Map.cs b/OpenRA.Game/Map/Map.cs index 2f578880cd..3f3f45a160 100644 --- a/OpenRA.Game/Map/Map.cs +++ b/OpenRA.Game/Map/Map.cs @@ -767,6 +767,9 @@ namespace OpenRA bool ContainsAllProjectedCellsCovering(MPos uv) { + if (MaximumTerrainHeight == 0) + return Contains((PPos)uv); + foreach (var puv in ProjectedCellsCovering(uv)) if (!Contains(puv)) return false; From 0739fc80a3f7f315080f4357e341b26a57bb361a Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Fri, 21 Aug 2015 20:24:59 +0100 Subject: [PATCH 10/10] Cache the speed modifiers enumerable, not just the traits, in Mobile. --- OpenRA.Mods.Common/Traits/Mobile.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Mobile.cs b/OpenRA.Mods.Common/Traits/Mobile.cs index 444cfac766..07f31d9023 100644 --- a/OpenRA.Mods.Common/Traits/Mobile.cs +++ b/OpenRA.Mods.Common/Traits/Mobile.cs @@ -310,7 +310,7 @@ namespace OpenRA.Mods.Common.Traits internal int TicksBeforePathing = 0; readonly Actor self; - readonly Lazy speedModifiers; + readonly Lazy> speedModifiers; public readonly MobileInfo Info; public bool IsMoving { get; set; } @@ -351,7 +351,7 @@ namespace OpenRA.Mods.Common.Traits self = init.Self; Info = info; - speedModifiers = Exts.Lazy(() => self.TraitsImplementing().ToArray()); + speedModifiers = Exts.Lazy(() => self.TraitsImplementing().ToArray().Select(x => x.GetSpeedModifier())); ToSubCell = FromSubCell = info.SharesCell ? init.World.Map.DefaultSubCell : SubCell.FullCell; if (init.Contains()) @@ -608,7 +608,7 @@ namespace OpenRA.Mods.Common.Traits if (terrainSpeed == 0) return 0; - var modifiers = speedModifiers.Value.Select(x => x.GetSpeedModifier()).Append(terrainSpeed); + var modifiers = speedModifiers.Value.Append(terrainSpeed); return Util.ApplyPercentageModifiers(Info.Speed, modifiers); }