From d67f696bd0bd73e1371217c6f69a95caa64ba02b Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 30 Jan 2022 16:43:22 +0000 Subject: [PATCH] Move BlockedByActor, IPositionableInfo, IPositionable to Mods.Common. Actor previously cached targetable locations for static actors as an optimization. As we can no longer reference the IPositionable interface, move this optimization to HitShape instead. Although we lose some of the efficiency of caching the final result on the actor, we gain some by allowing HitShape to cache the results as long as they have not changed. So instead of being limited to static actors, we can extend the caching to currently stationary actor. --- OpenRA.Game/Actor.cs | 18 +---- OpenRA.Game/Traits/TraitsInterfaces.cs | 74 ------------------- OpenRA.Mods.Common/Activities/Parachute.cs | 1 - .../Activities/SimpleTeleport.cs | 2 +- OpenRA.Mods.Common/Pathfinder/PathGraph.cs | 1 - OpenRA.Mods.Common/Pathfinder/PathSearch.cs | 1 - OpenRA.Mods.Common/Traits/HitShape.cs | 28 ++++++- OpenRA.Mods.Common/TraitsInterfaces.cs | 74 +++++++++++++++++++ 8 files changed, 103 insertions(+), 96 deletions(-) diff --git a/OpenRA.Game/Actor.cs b/OpenRA.Game/Actor.cs index f69e14b711..c447d35210 100644 --- a/OpenRA.Game/Actor.cs +++ b/OpenRA.Game/Actor.cs @@ -115,8 +115,7 @@ namespace OpenRA readonly INotifyBecomingIdle[] becomingIdles; readonly INotifyIdle[] tickIdles; readonly IEnumerable enabledTargetablePositions; - readonly bool setStaticTargetablePositions; - WPos[] staticTargetablePositions; + readonly IEnumerable enabledTargetableWorldPositions; bool created; internal Actor(World world, string name, TypeDictionary initDict) @@ -146,7 +145,6 @@ namespace OpenRA Info = world.Map.Rules.Actors[name]; - IPositionable positionable = null; var resolveOrdersList = new List(); var renderModifiersList = new List(); var rendersList = new List(); @@ -168,7 +166,6 @@ namespace OpenRA // performance-sensitive parts of the core game engine, such as pathfinding, visibility and rendering. // Note: The blocks are required to limit the scope of the t's, so we make an exception to our normal style // rules for spacing in order to keep these assignments compact and readable. - { if (trait is IPositionable t) positionable = t; } { if (trait is IOccupySpace t) OccupiesSpace = t; } { if (trait is IEffectiveOwner t) EffectiveOwner = t; } { if (trait is IFacing t) facing = t; } @@ -196,9 +193,8 @@ namespace OpenRA Targetables = targetablesList.ToArray(); var targetablePositions = targetablePositionsList.ToArray(); enabledTargetablePositions = targetablePositions.Where(Exts.IsTraitEnabled); + enabledTargetableWorldPositions = enabledTargetablePositions.SelectMany(tp => tp.TargetablePositions(this)); SyncHashes = syncHashesList.ToArray(); - - setStaticTargetablePositions = positionable == null && targetablePositions.Any() && targetablePositions.All(tp => tp.AlwaysEnabled); } } @@ -233,11 +229,6 @@ namespace OpenRA foreach (var notify in allObserverNotifiers) notify(this, readOnlyConditionCache); - // All actors that can move or teleport should have IPositionable, if not it's pretty safe to assume the actor is completely immobile and - // all targetable positions can be cached if all ITargetablePositions have no conditional requirements. - if (setStaticTargetablePositions) - staticTargetablePositions = enabledTargetablePositions.SelectMany(tp => tp.TargetablePositions(this)).ToArray(); - // TODO: Other traits may need initialization after being notified of initial condition state. // TODO: A post condition initialization notification phase may allow queueing activities instead. @@ -539,11 +530,8 @@ namespace OpenRA public IEnumerable GetTargetablePositions() { - if (staticTargetablePositions != null) - return staticTargetablePositions; - if (enabledTargetablePositions.Any()) - return enabledTargetablePositions.SelectMany(tp => tp.TargetablePositions(this)); + return enabledTargetableWorldPositions; return new[] { CenterPosition }; } diff --git a/OpenRA.Game/Traits/TraitsInterfaces.cs b/OpenRA.Game/Traits/TraitsInterfaces.cs index 5929b0baa3..3e79113e70 100644 --- a/OpenRA.Game/Traits/TraitsInterfaces.cs +++ b/OpenRA.Game/Traits/TraitsInterfaces.cs @@ -36,62 +36,6 @@ namespace OpenRA.Traits Dead = 32 } - /// - /// When performing locomotion or pathfinding related checks, - /// determines whether the blocking rules will be applied when encountering other actors. - /// - public enum BlockedByActor - { - /// - /// Actors on the map are ignored, as if they were not present. - /// An actor can only be blocked by impassable terrain. - /// An actor can never be blocked by other actors. The blocking rules will never be evaluated. - /// - None, - - /// - /// Actors on the map that are moving, or moveable & allied are ignored. - /// An actor is Immovable is any of the following applies: - /// - /// Lacks the Mobile trait. - /// The Mobile trait is disabled or paused. - /// The Mobile trait property IsImmovable is true. - /// - /// Note the above definition means an actor can be Movable, but may not be Moving, i.e. it is Stationary. - /// Actors are allied if their owners have the relationship. - /// An actor can be blocked by impassable terrain. - /// An actor can be blocked by immovable actors *if* they are deemed as blocking by the blocking rules. - /// An actor can be blocked by an actor capable of moving, if it is not an ally and *if* they are deemed as - /// blocking by the blocking rules. - /// An actor can never be blocked by an allied actor capable of moving, even if the other actor is stationary. - /// An actor can never be blocked by a moving actor. - /// - Immovable, - - /// - /// Actors on the map that are moving are ignored. - /// An actor is moving if both of the following apply: - /// - /// It is a Moveable actor (see ). - /// The Mobile trait property CurrentMovementTypes contains the flag Horizontal. - /// - /// Otherwise the actor is deemed to be Stationary. - /// An actor can be blocked by impassable terrain. - /// An actor can be blocked by immovable actors and stationary actors *if* they are deemed as blocking by the - /// blocking rules. - /// An actor can never be blocked by a moving actor. - /// - Stationary, - - /// - /// Actors on the map are not ignored. - /// An actor can be blocked by impassable terrain. - /// An actor can be blocked by immovable actors, stationary actors and moving actors *if* they are deemed as - /// blocking by the blocking rules. - /// - All - } - /// /// Type tag for DamageTypes . /// @@ -338,23 +282,6 @@ namespace OpenRA.Traits public enum SubCell : byte { Invalid = byte.MaxValue, Any = byte.MaxValue - 1, FullCell = 0, First = 1 } - public interface IPositionableInfo : IOccupySpaceInfo - { - bool CanEnterCell(World world, Actor self, CPos cell, SubCell subCell = SubCell.FullCell, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); - } - - public interface IPositionable : IOccupySpace - { - bool CanExistInCell(CPos location); - bool IsLeavingCell(CPos location, SubCell subCell = SubCell.Any); - bool CanEnterCell(CPos location, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); - SubCell GetValidSubCell(SubCell preferred = SubCell.Any); - SubCell GetAvailableSubCell(CPos location, SubCell preferredSubCell = SubCell.Any, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); - void SetPosition(Actor self, CPos cell, SubCell subCell = SubCell.Any); - void SetPosition(Actor self, WPos pos); - void SetCenterPosition(Actor self, WPos pos); - } - public interface ITemporaryBlockerInfo : ITraitInfoInterface { } [RequireExplicitImplementation] @@ -566,7 +493,6 @@ namespace OpenRA.Traits public interface ITargetablePositions { IEnumerable TargetablePositions(Actor self); - bool AlwaysEnabled { get; } } public interface IMoveInfo : ITraitInfoInterface diff --git a/OpenRA.Mods.Common/Activities/Parachute.cs b/OpenRA.Mods.Common/Activities/Parachute.cs index 329f2d25fd..22b46c87b3 100644 --- a/OpenRA.Mods.Common/Activities/Parachute.cs +++ b/OpenRA.Mods.Common/Activities/Parachute.cs @@ -11,7 +11,6 @@ using OpenRA.Activities; using OpenRA.Mods.Common.Traits; -using OpenRA.Traits; namespace OpenRA.Mods.Common.Activities { diff --git a/OpenRA.Mods.Common/Activities/SimpleTeleport.cs b/OpenRA.Mods.Common/Activities/SimpleTeleport.cs index 397ce87174..de03a7bc85 100644 --- a/OpenRA.Mods.Common/Activities/SimpleTeleport.cs +++ b/OpenRA.Mods.Common/Activities/SimpleTeleport.cs @@ -10,7 +10,7 @@ #endregion using OpenRA.Activities; -using OpenRA.Traits; +using OpenRA.Mods.Common.Traits; namespace OpenRA.Mods.Common.Activities { diff --git a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs index 14da1c20b3..9b6fda28c9 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathGraph.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathGraph.cs @@ -13,7 +13,6 @@ using System; using System.Collections.Generic; using System.Linq; using OpenRA.Mods.Common.Traits; -using OpenRA.Traits; namespace OpenRA.Mods.Common.Pathfinder { diff --git a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs index c677b50241..fb014cf6a3 100644 --- a/OpenRA.Mods.Common/Pathfinder/PathSearch.cs +++ b/OpenRA.Mods.Common/Pathfinder/PathSearch.cs @@ -15,7 +15,6 @@ using System.Linq; using System.Runtime.CompilerServices; using OpenRA.Mods.Common.Traits; using OpenRA.Primitives; -using OpenRA.Traits; namespace OpenRA.Mods.Common.Pathfinder { diff --git a/OpenRA.Mods.Common/Traits/HitShape.cs b/OpenRA.Mods.Common/Traits/HitShape.cs index fff76d530b..a224854a1b 100644 --- a/OpenRA.Mods.Common/Traits/HitShape.cs +++ b/OpenRA.Mods.Common/Traits/HitShape.cs @@ -74,6 +74,14 @@ namespace OpenRA.Mods.Common.Traits ITargetableCells targetableCells; Turreted turret; + ((CPos Cell, SubCell SubCell)[] targetableCells, + WPos? selfCenterPosition, + WRot? selfOrientation, + WRot? turretLocalOrientation, + WVec? turretOffset) cacheInput; + + WPos[] cachedTargetablePositions; + public HitShape(Actor self, HitShapeInfo info) : base(info) { @@ -88,14 +96,28 @@ namespace OpenRA.Mods.Common.Traits base.Created(self); } - bool ITargetablePositions.AlwaysEnabled => Info.RequiresCondition == null; - IEnumerable ITargetablePositions.TargetablePositions(Actor self) { if (IsTraitDisabled) return Enumerable.Empty(); - return TargetablePositions(self); + // Check for changes in inputs that affect the result of the TargetablePositions method. + // If the inputs have not changed we can cache and reuse the result for later calls. + // i.e. we are treating the method as a pure function. + var newCacheInput = ( + Info.UseTargetableCellsOffsets ? targetableCells?.TargetableCells() : null, + Info.TargetableOffsets.Length > 0 ? self.CenterPosition : (WPos?)null, + Info.TargetableOffsets.Length > 0 ? self.Orientation : (WRot?)null, + Info.TargetableOffsets.Length > 0 ? turret?.LocalOrientation : null, + Info.TargetableOffsets.Length > 0 ? turret?.Offset : null); + if (cachedTargetablePositions == null || + cacheInput != newCacheInput) + { + cachedTargetablePositions = TargetablePositions(self).ToArray(); + cacheInput = newCacheInput; + } + + return cachedTargetablePositions; } IEnumerable TargetablePositions(Actor self) diff --git a/OpenRA.Mods.Common/TraitsInterfaces.cs b/OpenRA.Mods.Common/TraitsInterfaces.cs index cce40590aa..49f47b3d88 100644 --- a/OpenRA.Mods.Common/TraitsInterfaces.cs +++ b/OpenRA.Mods.Common/TraitsInterfaces.cs @@ -715,4 +715,78 @@ namespace OpenRA.Mods.Common.Traits event Action CellEntryChanged; bool TryGetTerrainColorPair(MPos uv, out (Color Left, Color Right) value); } + + /// + /// When performing locomotion or pathfinding related checks, + /// determines whether the blocking rules will be applied when encountering other actors. + /// + public enum BlockedByActor + { + /// + /// Actors on the map are ignored, as if they were not present. + /// An actor can only be blocked by impassable terrain. + /// An actor can never be blocked by other actors. The blocking rules will never be evaluated. + /// + None, + + /// + /// Actors on the map that are moving, or moveable & allied are ignored. + /// An actor is Immovable is any of the following applies: + /// + /// Lacks the trait. + /// The trait has or + /// as true. + /// The trait has as true. + /// + /// Note the above definition means an actor can be Movable, but may not be Moving, i.e. it is Stationary. + /// Actors are allied if their owners have the relationship. + /// An actor can be blocked by impassable terrain. + /// An actor can be blocked by immovable actors *if* they are deemed as blocking by the blocking rules. + /// An actor can be blocked by an actor capable of moving, if it is not an ally and *if* they are deemed as + /// blocking by the blocking rules. + /// An actor can never be blocked by an allied actor capable of moving, even if the other actor is stationary. + /// An actor can never be blocked by a moving actor. + /// + Immovable, + + /// + /// Actors on the map that are moving are ignored. + /// An actor is moving if both of the following apply: + /// + /// It is a Moveable actor (see ). + /// contains the flag . + /// + /// Otherwise the actor is deemed to be Stationary. + /// An actor can be blocked by impassable terrain. + /// An actor can be blocked by immovable actors and stationary actors *if* they are deemed as blocking by the + /// blocking rules. + /// An actor can never be blocked by a moving actor. + /// + Stationary, + + /// + /// Actors on the map are not ignored. + /// An actor can be blocked by impassable terrain. + /// An actor can be blocked by immovable actors, stationary actors and moving actors *if* they are deemed as + /// blocking by the blocking rules. + /// + All + } + + public interface IPositionableInfo : IOccupySpaceInfo + { + bool CanEnterCell(World world, Actor self, CPos cell, SubCell subCell = SubCell.FullCell, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); + } + + public interface IPositionable : IOccupySpace + { + bool CanExistInCell(CPos location); + bool IsLeavingCell(CPos location, SubCell subCell = SubCell.Any); + bool CanEnterCell(CPos location, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); + SubCell GetValidSubCell(SubCell preferred = SubCell.Any); + SubCell GetAvailableSubCell(CPos location, SubCell preferredSubCell = SubCell.Any, Actor ignoreActor = null, BlockedByActor check = BlockedByActor.All); + void SetPosition(Actor self, CPos cell, SubCell subCell = SubCell.Any); + void SetPosition(Actor self, WPos pos); + void SetCenterPosition(Actor self, WPos pos); + } }