Merge pull request #10965 from RoosterDragon/actor-info-ordering
Strengthen trait ordering rules
This commit is contained in:
@@ -100,9 +100,15 @@ namespace OpenRA
|
||||
var unresolved = source.Except(resolved);
|
||||
|
||||
var testResolve = new Func<Type, Type, bool>((a, b) => a == b || a.IsAssignableFrom(b));
|
||||
var more = unresolved.Where(u => u.Dependencies.All(d => resolved.Exists(r => testResolve(d, r.Type))));
|
||||
|
||||
// Re-evaluate the vars above until sorted
|
||||
// This query detects which unresolved traits can be immediately resolved as all their direct dependencies are met.
|
||||
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.
|
||||
|
||||
// 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.
|
||||
while (more.Any())
|
||||
resolved.AddRange(more);
|
||||
|
||||
@@ -122,14 +128,14 @@ namespace OpenRA
|
||||
exceptionString += u.Type + ": { " + string.Join(", ", deps) + " }\r\n";
|
||||
}
|
||||
|
||||
throw new Exception(exceptionString);
|
||||
throw new YamlException(exceptionString);
|
||||
}
|
||||
|
||||
constructOrderCache = resolved.Select(r => r.Trait).ToList();
|
||||
return constructOrderCache;
|
||||
}
|
||||
|
||||
static IEnumerable<Type> PrerequisitesOf(ITraitInfo info)
|
||||
public static IEnumerable<Type> PrerequisitesOf(ITraitInfo info)
|
||||
{
|
||||
return info
|
||||
.GetType()
|
||||
|
||||
@@ -80,8 +80,11 @@ namespace OpenRA.Mods.Common.Traits
|
||||
void INotifyCreated.Created(Actor self)
|
||||
{
|
||||
upgradeManager = self.TraitOrDefault<UpgradeManager>();
|
||||
|
||||
// The upgrade manager exists, but may not have finished being created yet.
|
||||
// We'll defer the upgrades until the end of the tick, at which point it will be ready.
|
||||
if (Cloaked)
|
||||
GrantUpgrades(self);
|
||||
self.World.AddFrameEndTask(_ => GrantUpgrades(self));
|
||||
}
|
||||
|
||||
public bool Cloaked { get { return !IsTraitDisabled && remainingTime <= 0; } }
|
||||
|
||||
@@ -17,12 +17,18 @@ using OpenRA.Traits;
|
||||
namespace OpenRA.Mods.Common.Traits
|
||||
{
|
||||
[Desc("Attach this to a unit to enable dynamic upgrades by warheads, experience, crates, support powers, etc.")]
|
||||
public class UpgradeManagerInfo : ITraitInfo, Requires<IUpgradableInfo>
|
||||
public class UpgradeManagerInfo : TraitInfo<UpgradeManager>, IRulesetLoaded
|
||||
{
|
||||
public object Create(ActorInitializer init) { return new UpgradeManager(init); }
|
||||
public void RulesetLoaded(Ruleset rules, ActorInfo info)
|
||||
{
|
||||
if (!info.Name.StartsWith("^") && !info.TraitInfos<IUpgradableInfo>().Any())
|
||||
throw new YamlException(
|
||||
"There are no upgrades to be managed for actor '{0}'. You are either missing some upgradeable traits, or this UpgradeManager trait is not required.".F(
|
||||
info.Name));
|
||||
}
|
||||
}
|
||||
|
||||
public class UpgradeManager : ITick
|
||||
public class UpgradeManager : INotifyCreated, ITick
|
||||
{
|
||||
class TimedUpgrade
|
||||
{
|
||||
@@ -67,20 +73,21 @@ namespace OpenRA.Mods.Common.Traits
|
||||
}
|
||||
|
||||
readonly List<TimedUpgrade> timedUpgrades = new List<TimedUpgrade>();
|
||||
readonly Lazy<Dictionary<string, UpgradeState>> upgrades;
|
||||
readonly Dictionary<IUpgradable, int> levels = new Dictionary<IUpgradable, int>();
|
||||
Dictionary<string, UpgradeState> upgrades;
|
||||
|
||||
public UpgradeManager(ActorInitializer init)
|
||||
void INotifyCreated.Created(Actor self)
|
||||
{
|
||||
upgrades = Exts.Lazy(() =>
|
||||
{
|
||||
var ret = new Dictionary<string, UpgradeState>();
|
||||
foreach (var up in init.Self.TraitsImplementing<IUpgradable>())
|
||||
foreach (var t in up.UpgradeTypes)
|
||||
ret.GetOrAdd(t).Traits.Add(up);
|
||||
upgrades = new Dictionary<string, UpgradeState>();
|
||||
foreach (var up in self.TraitsImplementing<IUpgradable>())
|
||||
foreach (var t in up.UpgradeTypes)
|
||||
upgrades.GetOrAdd(t).Traits.Add(up);
|
||||
}
|
||||
|
||||
return ret;
|
||||
});
|
||||
void CheckCanManageUpgrades()
|
||||
{
|
||||
if (upgrades == null)
|
||||
throw new InvalidOperationException("Upgrades cannot be managed until the actor has been fully created.");
|
||||
}
|
||||
|
||||
/// <summary>Upgrade level increments are limited to dupesAllowed per source, i.e., if a single
|
||||
@@ -139,8 +146,10 @@ namespace OpenRA.Mods.Common.Traits
|
||||
|
||||
public void GrantUpgrade(Actor self, string upgrade, object source)
|
||||
{
|
||||
CheckCanManageUpgrades();
|
||||
|
||||
UpgradeState s;
|
||||
if (!upgrades.Value.TryGetValue(upgrade, out s))
|
||||
if (!upgrades.TryGetValue(upgrade, out s))
|
||||
return;
|
||||
|
||||
// Track the upgrade source so that the upgrade can be removed without conflicts
|
||||
@@ -151,8 +160,10 @@ namespace OpenRA.Mods.Common.Traits
|
||||
|
||||
public void RevokeUpgrade(Actor self, string upgrade, object source)
|
||||
{
|
||||
CheckCanManageUpgrades();
|
||||
|
||||
UpgradeState s;
|
||||
if (!upgrades.Value.TryGetValue(upgrade, out s))
|
||||
if (!upgrades.TryGetValue(upgrade, out s))
|
||||
return;
|
||||
|
||||
if (!s.Sources.Remove(source))
|
||||
@@ -164,14 +175,17 @@ namespace OpenRA.Mods.Common.Traits
|
||||
/// <summary>Returns true if the actor uses the given upgrade. Does not check the actual level of the upgrade.</summary>
|
||||
public bool AcknowledgesUpgrade(Actor self, string upgrade)
|
||||
{
|
||||
return upgrades.Value.ContainsKey(upgrade);
|
||||
CheckCanManageUpgrades();
|
||||
return upgrades.ContainsKey(upgrade);
|
||||
}
|
||||
|
||||
/// <summary>Returns true only if the actor can accept another level of the upgrade.</summary>
|
||||
public bool AcceptsUpgrade(Actor self, string upgrade)
|
||||
{
|
||||
CheckCanManageUpgrades();
|
||||
|
||||
UpgradeState s;
|
||||
if (!upgrades.Value.TryGetValue(upgrade, out s))
|
||||
if (!upgrades.TryGetValue(upgrade, out s))
|
||||
return false;
|
||||
|
||||
return s.Traits.Any(up => up.AcceptsUpgradeLevel(self, upgrade, GetOverallLevel(up) + 1));
|
||||
@@ -179,8 +193,10 @@ namespace OpenRA.Mods.Common.Traits
|
||||
|
||||
public void RegisterWatcher(string upgrade, Action<int, int> action)
|
||||
{
|
||||
CheckCanManageUpgrades();
|
||||
|
||||
UpgradeState s;
|
||||
if (!upgrades.Value.TryGetValue(upgrade, out s))
|
||||
if (!upgrades.TryGetValue(upgrade, out s))
|
||||
return;
|
||||
|
||||
s.Watchers.Add(action);
|
||||
@@ -192,6 +208,8 @@ namespace OpenRA.Mods.Common.Traits
|
||||
/// GrantTimedUpgrade).</summary>
|
||||
public void Tick(Actor self)
|
||||
{
|
||||
CheckCanManageUpgrades();
|
||||
|
||||
foreach (var u in timedUpgrades)
|
||||
{
|
||||
u.Tick();
|
||||
@@ -201,7 +219,7 @@ namespace OpenRA.Mods.Common.Traits
|
||||
|
||||
u.Sources.RemoveWhere(source => source.Remaining <= 0);
|
||||
|
||||
foreach (var a in upgrades.Value[u.Upgrade].Watchers)
|
||||
foreach (var a in upgrades[u.Upgrade].Watchers)
|
||||
a(u.Duration, u.Remaining);
|
||||
}
|
||||
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
#endregion
|
||||
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Text.RegularExpressions;
|
||||
using NUnit.Framework;
|
||||
@@ -28,89 +27,49 @@ namespace OpenRA.Test
|
||||
class MockEInfo : MockTraitInfo, Requires<MockFInfo> { }
|
||||
class MockFInfo : MockTraitInfo, Requires<MockDInfo> { }
|
||||
|
||||
class MockA2Info : MockTraitInfo { }
|
||||
class MockB2Info : MockTraitInfo { }
|
||||
class MockC2Info : MockTraitInfo { }
|
||||
|
||||
class MockStringInfo : MockTraitInfo { public string AString = null; }
|
||||
|
||||
[TestFixture]
|
||||
public class ActorInfoTest
|
||||
{
|
||||
[SetUp]
|
||||
public void SetUp()
|
||||
[TestCase(TestName = "Trait ordering sorts in dependency order correctly")]
|
||||
public void TraitOrderingSortsCorrectly()
|
||||
{
|
||||
var unorderedTraits = new ITraitInfo[] { new MockBInfo(), new MockCInfo(), new MockAInfo(), new MockBInfo() };
|
||||
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]);
|
||||
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 = "Sort traits in order of dependency")]
|
||||
public void TraitsInConstructOrderA()
|
||||
{
|
||||
var actorInfo = new ActorInfo("test", new MockCInfo(), new MockBInfo(), new MockAInfo());
|
||||
|
||||
var i = new List<ITraitInfo>(actorInfo.TraitsInConstructOrder());
|
||||
|
||||
Assert.That(i[0], Is.InstanceOf<MockAInfo>());
|
||||
Assert.That(i[1].GetType().Name, Is.EqualTo("MockBInfo"));
|
||||
Assert.That(i[2].GetType().Name, Is.EqualTo("MockCInfo"));
|
||||
}
|
||||
|
||||
[TestCase(TestName = "Exception reports missing dependencies")]
|
||||
public void TraitsInConstructOrderB()
|
||||
[TestCase(TestName = "Trait ordering exception reports missing dependencies")]
|
||||
public void TraitOrderingReportsMissingDependencies()
|
||||
{
|
||||
var actorInfo = new ActorInfo("test", new MockBInfo(), new MockCInfo());
|
||||
var ex = Assert.Throws<YamlException>(() => actorInfo.TraitsInConstructOrder());
|
||||
|
||||
try
|
||||
{
|
||||
actorInfo.TraitsInConstructOrder();
|
||||
throw new Exception("Exception not thrown!");
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
// Is.StringContaining is deprecated in NUnit 3, but we need to support NUnit 2 so we ignore the warning.
|
||||
#pragma warning disable CS0618
|
||||
Assert.That(e.Message, Is.StringContaining("MockA"));
|
||||
Assert.That(e.Message, Is.StringContaining("MockB"));
|
||||
Assert.That(e.Message, Is.StringContaining("MockC"));
|
||||
Assert.That(e.Message, Is.StringContaining("MockInherit"), "Should recognize base classes");
|
||||
Assert.That(e.Message, Is.StringContaining("IMock"), "Should recognize interfaces");
|
||||
#pragma warning restore CS0618
|
||||
}
|
||||
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 = "Exception reports cyclic dependencies")]
|
||||
public void TraitsInConstructOrderC()
|
||||
[TestCase(TestName = "Trait ordering exception reports cyclic dependencies")]
|
||||
public void TraitOrderingReportsCyclicDependencies()
|
||||
{
|
||||
var actorInfo = new ActorInfo("test", new MockDInfo(), new MockEInfo(), new MockFInfo());
|
||||
var ex = Assert.Throws<YamlException>(() => actorInfo.TraitsInConstructOrder());
|
||||
|
||||
try
|
||||
{
|
||||
actorInfo.TraitsInConstructOrder();
|
||||
throw new Exception("Exception not thrown!");
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
var count = (
|
||||
new Regex("MockD").Matches(e.Message).Count +
|
||||
new Regex("MockE").Matches(e.Message).Count +
|
||||
new Regex("MockF").Matches(e.Message).Count) / 3.0;
|
||||
|
||||
Assert.That(count, Is.EqualTo(Math.Floor(count)), "Should be symmetrical");
|
||||
}
|
||||
}
|
||||
|
||||
// This needs to match the logic used in RulesetCache.LoadYamlRules
|
||||
ActorInfo CreateActorInfoFromYaml(string name, string mapYaml, params string[] yamls)
|
||||
{
|
||||
var nodes = mapYaml == null ? new List<MiniYamlNode>() : MiniYaml.FromString(mapYaml);
|
||||
var sources = yamls.ToList();
|
||||
if (mapYaml != null)
|
||||
sources.Add(mapYaml);
|
||||
|
||||
var yaml = MiniYaml.Merge(sources.Select(s => MiniYaml.FromString(s)));
|
||||
var allUnits = yaml.ToDictionary(node => node.Key, node => node.Value);
|
||||
var unit = allUnits[name];
|
||||
var creator = new ObjectCreator(typeof(ActorInfoTest).Assembly);
|
||||
return new ActorInfo(creator, name, unit);
|
||||
StringAssert.Contains(typeof(MockDInfo).Name, ex.Message, "Exception message should report all cyclic dependencies.");
|
||||
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.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user