From 2583a7af316d5c4ca8b2cd5628714082ecb5e1f9 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 6 Feb 2022 10:19:31 +0000 Subject: [PATCH] After NotBefore<> support to control initialization order. Requires means that trait of type T will be initialized first, and asserts that at least one exists. The new NotBefore means that trait of type T will be initialized first, but allows no traits. This allows traits to control initialization order for optional dependencies. They want to be initialized second so they can rely on the dependencies having been initialized. But if the dependencies are optional then to not throw if none are present. We apply this to Locomotor which was previously using AddFrameEndTask to work around trait order initialization. This improves the user experience as the initialization is applied whilst the loading screen is still visible, rather than the game starting and creating jank by performing initialization on the first tick. --- OpenRA.Game/GameRules/ActorInfo.cs | 22 ++++++-- OpenRA.Game/Traits/TraitsInterfaces.cs | 3 + .../Traits/World/ElevatedBridgeLayer.cs | 2 +- .../Traits/World/JumpjetActorLayer.cs | 2 +- OpenRA.Mods.Common/Traits/World/Locomotor.cs | 43 +++++++------- .../Traits/World/SubterraneanActorLayer.cs | 2 +- .../Traits/World/TerrainTunnelLayer.cs | 2 +- OpenRA.Mods.Common/TraitsInterfaces.cs | 2 + OpenRA.Test/OpenRA.Game/ActorInfoTest.cs | 56 ++++++++++++++++++- 9 files changed, 100 insertions(+), 34 deletions(-) diff --git a/OpenRA.Game/GameRules/ActorInfo.cs b/OpenRA.Game/GameRules/ActorInfo.cs index e0215f815c..c856289732 100644 --- a/OpenRA.Game/GameRules/ActorInfo.cs +++ b/OpenRA.Game/GameRules/ActorInfo.cs @@ -110,10 +110,11 @@ namespace OpenRA { Trait = i, Type = i.GetType(), - Dependencies = PrerequisitesOf(i).ToList() + Dependencies = PrerequisitesOf(i).ToList(), + OptionalDependencies = OptionalPrerequisitesOf(i).ToList() }).ToList(); - var resolved = source.Where(s => !s.Dependencies.Any()).ToList(); + var resolved = source.Where(s => !s.Dependencies.Any() && !s.OptionalDependencies.Any()).ToList(); var unresolved = source.Except(resolved); var testResolve = new Func((a, b) => a == b || a.IsAssignableFrom(b)); @@ -122,7 +123,9 @@ namespace OpenRA var more = unresolved.Where(u => u.Dependencies.All(d => // To be resolvable, all dependencies must be satisfied according to the following conditions: resolved.Exists(r => testResolve(d, r.Type)) && // There must exist a resolved trait that meets the dependency. - !unresolved.Any(u1 => testResolve(d, u1.Type)))); // All matching traits that meet this dependency must be resolved first. + !unresolved.Any(u1 => testResolve(d, u1.Type))) && // All matching traits that meet this dependency must be resolved first. + u.OptionalDependencies.All(d => // To be resolvable, all optional dependencies must be satisfied according to the following condition: + !unresolved.Any(u1 => testResolve(d, u1.Type)))); // All matching traits that meet this optional dependencies must be resolved first. // Continue resolving traits as long as possible. // Each time we resolve some traits, this means dependencies for other traits may then be possible to satisfy in the next pass. @@ -142,7 +145,9 @@ namespace OpenRA foreach (var u in unresolved) { var deps = u.Dependencies.Where(d => !resolved.Exists(r => r.Type == d)); - exceptionString += u.Type + ": { " + string.Join(", ", deps) + " }\r\n"; + var optDeps = u.OptionalDependencies.Where(d => !resolved.Exists(r => r.Type == d)); + var allDeps = string.Join(", ", deps.Select(o => o.ToString()).Concat(optDeps.Select(o => $"[{o}]"))); + exceptionString += $"{u.Type}: {{ {allDeps} }}\r\n"; } throw new YamlException(exceptionString); @@ -161,6 +166,15 @@ namespace OpenRA .Select(t => t.GetGenericArguments()[0]); } + public static IEnumerable OptionalPrerequisitesOf(TraitInfo info) + { + return info + .GetType() + .GetInterfaces() + .Where(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(NotBefore<>)) + .Select(t => t.GetGenericArguments()[0]); + } + public bool HasTraitInfo() where T : ITraitInfoInterface { return traits.Contains(); } public T TraitInfo() where T : ITraitInfoInterface { return traits.Get(); } public T TraitInfoOrDefault() where T : ITraitInfoInterface { return traits.GetOrDefault(); } diff --git a/OpenRA.Game/Traits/TraitsInterfaces.cs b/OpenRA.Game/Traits/TraitsInterfaces.cs index 1c558b48d5..20cf5caff0 100644 --- a/OpenRA.Game/Traits/TraitsInterfaces.cs +++ b/OpenRA.Game/Traits/TraitsInterfaces.cs @@ -322,6 +322,9 @@ namespace OpenRA.Traits [SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Not a real interface, but more like a tag.")] public interface Requires where T : class, ITraitInfoInterface { } + [SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Not a real interface, but more like a tag.")] + public interface NotBefore where T : class, ITraitInfoInterface { } + public interface IActivityInterface { } [RequireExplicitImplementation] diff --git a/OpenRA.Mods.Common/Traits/World/ElevatedBridgeLayer.cs b/OpenRA.Mods.Common/Traits/World/ElevatedBridgeLayer.cs index 5d170a8f31..f042eeebf0 100644 --- a/OpenRA.Mods.Common/Traits/World/ElevatedBridgeLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/ElevatedBridgeLayer.cs @@ -17,7 +17,7 @@ using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits { [TraitLocation(SystemActors.World)] - public class ElevatedBridgeLayerInfo : TraitInfo, Requires, ILobbyCustomRulesIgnore + public class ElevatedBridgeLayerInfo : TraitInfo, Requires, ILobbyCustomRulesIgnore, ICustomMovementLayerInfo { [Desc("Terrain type used by cells outside any elevated bridge footprint.")] public readonly string ImpassableTerrainType = "Impassable"; diff --git a/OpenRA.Mods.Common/Traits/World/JumpjetActorLayer.cs b/OpenRA.Mods.Common/Traits/World/JumpjetActorLayer.cs index 64323b7c70..59d08c69be 100644 --- a/OpenRA.Mods.Common/Traits/World/JumpjetActorLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/JumpjetActorLayer.cs @@ -16,7 +16,7 @@ using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits { [TraitLocation(SystemActors.World)] - public class JumpjetActorLayerInfo : TraitInfo + public class JumpjetActorLayerInfo : TraitInfo, ICustomMovementLayerInfo { [Desc("Terrain type of the airborne layer.")] public readonly string TerrainType = "Jumpjet"; diff --git a/OpenRA.Mods.Common/Traits/World/Locomotor.cs b/OpenRA.Mods.Common/Traits/World/Locomotor.cs index 14115df411..dee991f370 100644 --- a/OpenRA.Mods.Common/Traits/World/Locomotor.cs +++ b/OpenRA.Mods.Common/Traits/World/Locomotor.cs @@ -57,7 +57,7 @@ namespace OpenRA.Mods.Common.Traits [TraitLocation(SystemActors.World | SystemActors.EditorWorld)] [Desc("Used by Mobile. Attach these to the world actor. You can have multiple variants by adding @suffixes.")] - public class LocomotorInfo : TraitInfo + public class LocomotorInfo : TraitInfo, NotBefore { [Desc("Locomotor ID.")] public readonly string Name = "default"; @@ -376,34 +376,31 @@ namespace OpenRA.Mods.Common.Traits map.CustomTerrain.CellEntryChanged += UpdateCellCost; map.Tiles.CellEntryChanged += UpdateCellCost; - // This section needs to run after WorldLoaded() because we need to be sure that all types of ICustomMovementLayer have been initialized. - w.AddFrameEndTask(_ => + // NotBefore<> ensures all custom movement layers have been initialized. + var customMovementLayers = world.GetCustomMovementLayers(); + Array.Resize(ref cellsCost, customMovementLayers.Length); + Array.Resize(ref blockingCache, customMovementLayers.Length); + foreach (var cml in customMovementLayers) { - var customMovementLayers = world.GetCustomMovementLayers(); - Array.Resize(ref cellsCost, customMovementLayers.Length); - Array.Resize(ref blockingCache, customMovementLayers.Length); - foreach (var cml in customMovementLayers) + if (cml == null) + continue; + + var cellLayer = new CellLayer(map); + cellsCost[cml.Index] = cellLayer; + blockingCache[cml.Index] = new CellLayer(map); + + foreach (var cell in map.AllCells) { - if (cml == null) - continue; + var index = cml.GetTerrainIndex(cell); - var cellLayer = new CellLayer(map); - cellsCost[cml.Index] = cellLayer; - blockingCache[cml.Index] = new CellLayer(map); + var cost = PathGraph.MovementCostForUnreachableCell; - foreach (var cell in map.AllCells) - { - var index = cml.GetTerrainIndex(cell); + if (index != byte.MaxValue) + cost = terrainInfos[index].Cost; - var cost = PathGraph.MovementCostForUnreachableCell; - - if (index != byte.MaxValue) - cost = terrainInfos[index].Cost; - - cellLayer[cell] = cost; - } + cellLayer[cell] = cost; } - }); + } } CellCache GetCache(CPos cell) diff --git a/OpenRA.Mods.Common/Traits/World/SubterraneanActorLayer.cs b/OpenRA.Mods.Common/Traits/World/SubterraneanActorLayer.cs index 7bd786ecc1..bd6ca6f025 100644 --- a/OpenRA.Mods.Common/Traits/World/SubterraneanActorLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/SubterraneanActorLayer.cs @@ -16,7 +16,7 @@ using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits { [TraitLocation(SystemActors.World)] - public class SubterraneanActorLayerInfo : TraitInfo + public class SubterraneanActorLayerInfo : TraitInfo, ICustomMovementLayerInfo { [Desc("Terrain type of the underground layer.")] public readonly string TerrainType = "Subterranean"; diff --git a/OpenRA.Mods.Common/Traits/World/TerrainTunnelLayer.cs b/OpenRA.Mods.Common/Traits/World/TerrainTunnelLayer.cs index 8710be2e33..ce11dbff20 100644 --- a/OpenRA.Mods.Common/Traits/World/TerrainTunnelLayer.cs +++ b/OpenRA.Mods.Common/Traits/World/TerrainTunnelLayer.cs @@ -17,7 +17,7 @@ using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits { [TraitLocation(SystemActors.World)] - public class TerrainTunnelLayerInfo : TraitInfo, Requires, ILobbyCustomRulesIgnore + public class TerrainTunnelLayerInfo : TraitInfo, Requires, ILobbyCustomRulesIgnore, ICustomMovementLayerInfo { [Desc("Terrain type used by cells outside any tunnel footprint.")] public readonly string ImpassableTerrainType = "Impassable"; diff --git a/OpenRA.Mods.Common/TraitsInterfaces.cs b/OpenRA.Mods.Common/TraitsInterfaces.cs index 49f47b3d88..c5610e60c7 100644 --- a/OpenRA.Mods.Common/TraitsInterfaces.cs +++ b/OpenRA.Mods.Common/TraitsInterfaces.cs @@ -411,6 +411,8 @@ namespace OpenRA.Mods.Common.Traits WPos CenterOfCell(CPos cell); } + public interface ICustomMovementLayerInfo : ITraitInfoInterface { } + // For traits that want to be exposed to the "Deploy" UI button / hotkey [RequireExplicitImplementation] public interface IIssueDeployOrder diff --git a/OpenRA.Test/OpenRA.Game/ActorInfoTest.cs b/OpenRA.Test/OpenRA.Game/ActorInfoTest.cs index aa1135cacf..9d06b4ed67 100644 --- a/OpenRA.Test/OpenRA.Game/ActorInfoTest.cs +++ b/OpenRA.Test/OpenRA.Game/ActorInfoTest.cs @@ -18,13 +18,23 @@ namespace OpenRA.Test interface IMock : ITraitInfoInterface { } class MockTraitInfo : TraitInfo { public override object Create(ActorInitializer init) { return null; } } class MockInheritInfo : MockTraitInfo { } + class MockAInfo : MockInheritInfo, IMock { } - class MockBInfo : MockTraitInfo, Requires, Requires, Requires { } + class MockBInfo : MockTraitInfo, Requires, Requires { } class MockCInfo : MockTraitInfo, Requires { } + class MockDInfo : MockTraitInfo, Requires { } class MockEInfo : MockTraitInfo, Requires { } class MockFInfo : MockTraitInfo, Requires { } + class MockGInfo : MockInheritInfo, IMock, NotBefore { } + class MockHInfo : MockTraitInfo, NotBefore, NotBefore, NotBefore { } + class MockIInfo : MockTraitInfo, NotBefore, NotBefore { } + + class MockJInfo : MockTraitInfo, NotBefore { } + class MockKInfo : MockTraitInfo, NotBefore { } + class MockLInfo : MockTraitInfo, NotBefore { } + [TestFixture] public class ActorInfoTest { @@ -39,7 +49,27 @@ namespace OpenRA.Test for (var i = 0; i < orderedTraits.Length; i++) { - var traitTypesThatMustOccurBeforeThisTrait = ActorInfo.PrerequisitesOf(orderedTraits[i]); + var traitTypesThatMustOccurBeforeThisTrait = + ActorInfo.PrerequisitesOf(orderedTraits[i]).Concat(ActorInfo.OptionalPrerequisitesOf(orderedTraits[i])); + var traitTypesThatOccurAfterThisTrait = orderedTraits.Skip(i + 1).Select(ti => ti.GetType()); + var traitTypesThatShouldOccurEarlier = traitTypesThatOccurAfterThisTrait.Intersect(traitTypesThatMustOccurBeforeThisTrait); + CollectionAssert.IsEmpty(traitTypesThatShouldOccurEarlier, "Dependency order has not been satisfied."); + } + } + + [TestCase(TestName = "Trait ordering sorts in optional dependency order correctly")] + public void OptionalTraitOrderingSortsCorrectly() + { + var unorderedTraits = new TraitInfo[] { new MockHInfo(), new MockIInfo(), new MockGInfo(), new MockHInfo() }; + var actorInfo = new ActorInfo("test", unorderedTraits); + var orderedTraits = actorInfo.TraitsInConstructOrder().ToArray(); + + CollectionAssert.AreEquivalent(unorderedTraits, orderedTraits); + + for (var i = 0; i < orderedTraits.Length; i++) + { + var traitTypesThatMustOccurBeforeThisTrait = + ActorInfo.PrerequisitesOf(orderedTraits[i]).Concat(ActorInfo.OptionalPrerequisitesOf(orderedTraits[i])); var traitTypesThatOccurAfterThisTrait = orderedTraits.Skip(i + 1).Select(ti => ti.GetType()); var traitTypesThatShouldOccurEarlier = traitTypesThatOccurAfterThisTrait.Intersect(traitTypesThatMustOccurBeforeThisTrait); CollectionAssert.IsEmpty(traitTypesThatShouldOccurEarlier, "Dependency order has not been satisfied."); @@ -52,13 +82,22 @@ namespace OpenRA.Test var actorInfo = new ActorInfo("test", new MockBInfo(), new MockCInfo()); var ex = Assert.Throws(() => actorInfo.TraitsInConstructOrder()); - StringAssert.Contains(typeof(MockAInfo).Name, ex.Message, "Exception message did not report a missing dependency."); StringAssert.Contains(typeof(MockBInfo).Name, ex.Message, "Exception message did not report a missing dependency."); StringAssert.Contains(typeof(MockCInfo).Name, ex.Message, "Exception message did not report a missing dependency."); StringAssert.Contains(typeof(MockInheritInfo).Name, ex.Message, "Exception message did not report a missing dependency (from a base class)."); StringAssert.Contains(typeof(IMock).Name, ex.Message, "Exception message did not report a missing dependency (from an interface)."); } + [TestCase(TestName = "Trait ordering allows optional dependencies to be missing")] + public void TraitOrderingAllowsMissingOptionalDependencies() + { + var unorderedTraits = new TraitInfo[] { new MockHInfo(), new MockIInfo() }; + var actorInfo = new ActorInfo("test", unorderedTraits); + var orderedTraits = actorInfo.TraitsInConstructOrder().ToArray(); + + CollectionAssert.AreEquivalent(unorderedTraits, orderedTraits); + } + [TestCase(TestName = "Trait ordering exception reports cyclic dependencies")] public void TraitOrderingReportsCyclicDependencies() { @@ -69,5 +108,16 @@ namespace OpenRA.Test StringAssert.Contains(typeof(MockEInfo).Name, ex.Message, "Exception message should report all cyclic dependencies."); StringAssert.Contains(typeof(MockFInfo).Name, ex.Message, "Exception message should report all cyclic dependencies."); } + + [TestCase(TestName = "Trait ordering exception reports cyclic optional dependencies")] + public void TraitOrderingReportsCyclicOptionalDependencies() + { + var actorInfo = new ActorInfo("test", new MockJInfo(), new MockKInfo(), new MockLInfo()); + var ex = Assert.Throws(() => actorInfo.TraitsInConstructOrder()); + + StringAssert.Contains(typeof(MockJInfo).Name, ex.Message, "Exception message should report all cyclic dependencies."); + StringAssert.Contains(typeof(MockKInfo).Name, ex.Message, "Exception message should report all cyclic dependencies."); + StringAssert.Contains(typeof(MockLInfo).Name, ex.Message, "Exception message should report all cyclic dependencies."); + } } }