After NotBefore<> support to control initialization order.

Requires<T> means that trait of type T will be initialized first, and asserts that at least one exists. The new NotBefore<T> 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

@@ -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<Type, Type, bool>((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<Type> OptionalPrerequisitesOf(TraitInfo info)
+		{
+			return info
+				.GetType()
+				.GetInterfaces()
+				.Where(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(NotBefore<>))
+				.Select(t => t.GetGenericArguments()[0]);
+		}
+
 		public bool HasTraitInfo<T>() where T : ITraitInfoInterface { return traits.Contains<T>(); }
 		public T TraitInfo<T>() where T : ITraitInfoInterface { return traits.Get<T>(); }
 		public T TraitInfoOrDefault<T>() where T : ITraitInfoInterface { return traits.GetOrDefault<T>(); }

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<T> where T : class, ITraitInfoInterface { }
 
+	[SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Not a real interface, but more like a tag.")]
+	public interface NotBefore<T> where T : class, ITraitInfoInterface { }
+
 	public interface IActivityInterface { }
 
 	[RequireExplicitImplementation]

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<DomainIndexInfo>, ILobbyCustomRulesIgnore
+	public class ElevatedBridgeLayerInfo : TraitInfo, Requires<DomainIndexInfo>, ILobbyCustomRulesIgnore, ICustomMovementLayerInfo
 	{
 		[Desc("Terrain type used by cells outside any elevated bridge footprint.")]
 		public readonly string ImpassableTerrainType = "Impassable";

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";

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<ICustomMovementLayerInfo>
 	{
 		[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.
+			// NotBefore<> ensures all custom movement layers have been initialized.
-			w.AddFrameEndTask(_ =>
+			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();
+				if (cml == null)
-				Array.Resize(ref cellsCost, customMovementLayers.Length);
+					continue;
-				Array.Resize(ref blockingCache, customMovementLayers.Length);
+
-				foreach (var cml in customMovementLayers)
+				var cellLayer = new CellLayer<short>(map);
+				cellsCost[cml.Index] = cellLayer;
+				blockingCache[cml.Index] = new CellLayer<CellCache>(map);
+
+				foreach (var cell in map.AllCells)
 				{
-					if (cml == null)
+					var index = cml.GetTerrainIndex(cell);
-						continue;
 
-					var cellLayer = new CellLayer<short>(map);
+					var cost = PathGraph.MovementCostForUnreachableCell;
-					cellsCost[cml.Index] = cellLayer;
-					blockingCache[cml.Index] = new CellLayer<CellCache>(map);
 
-					foreach (var cell in map.AllCells)
+					if (index != byte.MaxValue)
-					{
+						cost = terrainInfos[index].Cost;
-						var index = cml.GetTerrainIndex(cell);
 
-						var cost = PathGraph.MovementCostForUnreachableCell;
+					cellLayer[cell] = cost;
-
-						if (index != byte.MaxValue)
-							cost = terrainInfos[index].Cost;
-
-						cellLayer[cell] = cost;
-					}
 				}
-			});
+			}
 		}
 
 		CellCache GetCache(CPos cell)

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";

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<DomainIndexInfo>, ILobbyCustomRulesIgnore
+	public class TerrainTunnelLayerInfo : TraitInfo, Requires<DomainIndexInfo>, ILobbyCustomRulesIgnore, ICustomMovementLayerInfo
 	{
 		[Desc("Terrain type used by cells outside any tunnel footprint.")]
 		public readonly string ImpassableTerrainType = "Impassable";

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

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<MockAInfo>, Requires<IMock>, Requires<MockInheritInfo> { }
+	class MockBInfo : MockTraitInfo, Requires<IMock>, Requires<MockInheritInfo> { }
 	class MockCInfo : MockTraitInfo, Requires<MockBInfo> { }
+
 	class MockDInfo : MockTraitInfo, Requires<MockEInfo> { }
 	class MockEInfo : MockTraitInfo, Requires<MockFInfo> { }
 	class MockFInfo : MockTraitInfo, Requires<MockDInfo> { }
 
+	class MockGInfo : MockInheritInfo, IMock, NotBefore<MockAInfo> { }
+	class MockHInfo : MockTraitInfo, NotBefore<IMock>, NotBefore<MockInheritInfo>, NotBefore<MockBInfo> { }
+	class MockIInfo : MockTraitInfo, NotBefore<MockHInfo>, NotBefore<MockCInfo> { }
+
+	class MockJInfo : MockTraitInfo, NotBefore<MockKInfo> { }
+	class MockKInfo : MockTraitInfo, NotBefore<MockLInfo> { }
+	class MockLInfo : MockTraitInfo, NotBefore<MockJInfo> { }
+
 	[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<YamlException>(() => 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<YamlException>(() => 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.");
+		}
 	}
 }