Merge pull request #8000 from pchote/bogus-yaml-removals

Fix yaml merging
This commit is contained in:
Matthias Mailänder
2015-04-25 13:42:18 +02:00
8 changed files with 156 additions and 254 deletions

View File

@@ -16,15 +16,18 @@ using OpenRA.Traits;
namespace OpenRA
{
// TODO: This is not exported into the documentation yet.
[Desc("A unit/building inside the game. Every rules starts with one and adds trait to it.",
"Special actors like world or player are usually defined in system.yaml and affect everything.")]
/// <summary>
/// A unit/building inside the game. Every rules starts with one and adds trait to it.
/// Special actors like world or player are usually defined in system.yaml and affect everything.
/// </summary>
public class ActorInfo
{
[Desc("The actor name can be anything, but the sprites used in the Render*: traits default to this one.",
"If you add an ^ in front of the name, the engine will recognize this as a collection of traits",
"that can be inherited by others (using Inherits:) and not a real unit.",
"You can remove inherited traits by adding a - infront of them as in -TraitName: to inherit everything, but this trait.")]
/// <summary>
/// The actor name can be anything, but the sprites used in the Render*: traits default to this one.
/// If you add an ^ in front of the name, the engine will recognize this as a collection of traits
/// that can be inherited by others (using Inherits:) and not a real unit.
/// You can remove inherited traits by adding a - infront of them as in -TraitName: to inherit everything, but this trait.
/// </summary>
public readonly string Name;
public readonly TypeDictionary Traits = new TypeDictionary();
List<ITraitInfo> constructOrderCache = null;
@@ -33,12 +36,22 @@ namespace OpenRA
{
try
{
var mergedNode = MergeWithParent(node, allUnits).ToDictionary();
var allParents = new HashSet<string>();
// Guard against circular inheritance
allParents.Add(name);
var mergedNode = MergeWithParents(node, allUnits, allParents).ToDictionary();
Name = name;
foreach (var t in mergedNode)
if (t.Key != "Inherits" && !t.Key.StartsWith("-"))
{
if (t.Key[0] == '-')
throw new YamlException("Bogus trait removal: " + t.Key);
if (t.Key != "Inherits" && !t.Key.StartsWith("Inherits@"))
Traits.Add(LoadTraitInfo(t.Key.Split('@')[0], t.Value));
}
}
catch (YamlException e)
{
@@ -46,32 +59,31 @@ namespace OpenRA
}
}
static MiniYaml GetParent(MiniYaml node, Dictionary<string, MiniYaml> allUnits)
static Dictionary<string, MiniYaml> GetParents(MiniYaml node, Dictionary<string, MiniYaml> allUnits)
{
MiniYaml inherits;
node.ToDictionary().TryGetValue("Inherits", out inherits);
if (inherits == null || string.IsNullOrEmpty(inherits.Value))
return null;
return node.Nodes.Where(n => n.Key == "Inherits" || n.Key.StartsWith("Inherits@"))
.ToDictionary(n => n.Value.Value, n =>
{
MiniYaml i;
if (!allUnits.TryGetValue(n.Value.Value, out i))
throw new YamlException(
"Bogus inheritance -- parent type {0} does not exist".F(n.Value.Value));
MiniYaml parent;
allUnits.TryGetValue(inherits.Value, out parent);
if (parent == null)
throw new InvalidOperationException(
"Bogus inheritance -- actor type {0} does not exist".F(inherits.Value));
return parent;
return i;
});
}
static MiniYaml MergeWithParent(MiniYaml node, Dictionary<string, MiniYaml> allUnits)
static MiniYaml MergeWithParents(MiniYaml node, Dictionary<string, MiniYaml> allUnits, HashSet<string> allParents)
{
var parent = GetParent(node, allUnits);
if (parent != null)
{
var result = MiniYaml.MergeStrict(node, MergeWithParent(parent, allUnits));
var parents = GetParents(node, allUnits);
// strip the '-'
result.Nodes.RemoveAll(a => a.Key.StartsWith("-"));
return result;
foreach (var kv in parents)
{
if (!allParents.Add(kv.Key))
throw new YamlException(
"Bogus inheritance -- duplicate inheritance of {0}.".F(kv.Key));
node = MiniYaml.MergeStrict(node, MergeWithParents(kv.Value, allUnits, allParents));
}
return node;

53
OpenRA.Game/MiniYaml.cs Executable file → Normal file
View File

@@ -254,17 +254,17 @@ namespace OpenRA
return FromLines(text.Split(new[] { "\r\n", "\n" }, StringSplitOptions.RemoveEmptyEntries), fileName);
}
public static List<MiniYamlNode> MergeLiberal(List<MiniYamlNode> a, List<MiniYamlNode> b)
public static List<MiniYamlNode> MergeStrict(List<MiniYamlNode> a, List<MiniYamlNode> b)
{
return Merge(a, b, false);
}
public static List<MiniYamlNode> MergeStrict(List<MiniYamlNode> a, List<MiniYamlNode> b)
public static List<MiniYamlNode> MergeLiberal(List<MiniYamlNode> a, List<MiniYamlNode> b)
{
return Merge(a, b, true);
}
static List<MiniYamlNode> Merge(List<MiniYamlNode> a, List<MiniYamlNode> b, bool throwErrors)
static List<MiniYamlNode> Merge(List<MiniYamlNode> a, List<MiniYamlNode> b, bool allowUnresolvedRemoves = false)
{
if (a.Count == 0)
return b;
@@ -275,10 +275,11 @@ namespace OpenRA
var dictA = a.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => "{0} (at {1})".F(x.Key, x.Location));
var dictB = b.ToDictionaryWithConflictLog(x => x.Key, "MiniYaml.Merge", null, x => "{0} (at {1})".F(x.Key, x.Location));
var keys = dictA.Keys.Union(dictB.Keys).ToList();
var allKeys = dictA.Keys.Union(dictB.Keys);
var noInherit = keys.Where(x => x.Length > 0 && x[0] == '-')
.ToDictionary(x => x.Substring(1), x => false);
var keys = allKeys.Where(x => x.Length == 0 || x[0] != '-').ToList();
var removeKeys = allKeys.Where(x => x.Length > 0 && x[0] == '-')
.Select(k => k.Substring(1)).ToHashSet();
foreach (var key in keys)
{
@@ -286,47 +287,55 @@ namespace OpenRA
dictA.TryGetValue(key, out aa);
dictB.TryGetValue(key, out bb);
if (noInherit.ContainsKey(key))
{
if (!throwErrors)
if (aa != null)
ret.Add(aa);
noInherit[key] = true;
}
if (removeKeys.Contains(key))
removeKeys.Remove(key);
else
{
var loc = aa == null ? default(MiniYamlNode.SourceLocation) : aa.Location;
var merged = (aa == null || bb == null) ? aa ?? bb : new MiniYamlNode(key, Merge(aa.Value, bb.Value, throwErrors), loc);
var merged = (aa == null || bb == null) ? aa ?? bb : new MiniYamlNode(key, Merge(aa.Value, bb.Value, allowUnresolvedRemoves), loc);
ret.Add(merged);
}
}
if (throwErrors && noInherit.ContainsValue(false))
throw new YamlException("Bogus yaml removals: {0}".F(
noInherit.Where(x => !x.Value).JoinWith(", ")));
if (removeKeys.Any())
{
if (allowUnresolvedRemoves)
{
// Add the removal nodes back for the next pass to deal with
foreach (var k in removeKeys)
{
var key = "-" + k;
MiniYamlNode rem;
if (!dictA.TryGetValue(key, out rem))
rem = dictB[key];
ret.Add(rem);
}
}
else
throw new YamlException("Bogus yaml removals: {0}".F(removeKeys.JoinWith(", ")));
}
return ret;
}
public static MiniYaml MergeLiberal(MiniYaml a, MiniYaml b)
{
return Merge(a, b, false);
return Merge(a, b, true);
}
public static MiniYaml MergeStrict(MiniYaml a, MiniYaml b)
{
return Merge(a, b, true);
return Merge(a, b, false);
}
static MiniYaml Merge(MiniYaml a, MiniYaml b, bool throwErrors)
static MiniYaml Merge(MiniYaml a, MiniYaml b, bool allowUnresolvedRemoves)
{
if (a == null)
return b;
if (b == null)
return a;
return new MiniYaml(a.Value ?? b.Value, Merge(a.Nodes, b.Nodes, throwErrors));
return new MiniYaml(a.Value ?? b.Value, Merge(a.Nodes, b.Nodes, allowUnresolvedRemoves));
}
public IEnumerable<string> ToLines(string name)

View File

@@ -21,17 +21,22 @@ namespace OpenRA.Test
[TestFixture]
public class MiniYamlTest
{
readonly string yamlForParent = @"
^Parent:
FromParent:
FromParentRemove:
readonly string mixedMergeA = @"
Merge:
FromA:
FromARemovedB:
FromARemovedA:
-FromBRemovedA:
-FromARemovedA:
";
readonly string yamlForChild = @"
Child:
Inherits: ^Parent
FromChild:
-FromParentRemove:
readonly string mixedMergeB = @"
Merge:
FromB:
FromBRemovedA:
FromBRemovedB:
-FromARemovedB:
-FromBRemovedB:
";
readonly string yamlTabStyle = @"
@@ -60,58 +65,28 @@ Root2:
Attribute1: Test
";
List<MiniYamlNode> parentList;
List<MiniYamlNode> childList;
MiniYaml parent;
MiniYaml child;
[SetUp]
public void SetUp()
{
parentList = MiniYaml.FromString(yamlForParent);
childList = MiniYaml.FromString(yamlForChild);
parent = parentList.First().Value;
child = childList.First().Value;
}
void InheritanceTest(List<MiniYamlNode> nodes)
{
Assert.That(nodes.Any(n => n.Key == "FromParent"), Is.True, "Node from parent");
Assert.That(nodes.Any(n => n.Key == "FromChild"), Is.True, "Node from child");
Assert.That(nodes.Any(n => n.Key == "FromParentRemove"), Is.Not.True, "Node from parent - node from child");
}
[TestCase(TestName = "MergeStrict(MiniYaml, MiniYaml)")]
[TestCase(TestName = "Merging: mixed addition and removal")]
public void MergeYamlA()
{
var res = MiniYaml.MergeStrict(parent, child);
InheritanceTest(res.Nodes);
var a = MiniYaml.FromString(mixedMergeA, "mixedMergeA");
var b = MiniYaml.FromString(mixedMergeB, "mixedMergeB");
// Merge order should not matter
// Note: All the Merge* variants are different plumbing over the same
// internal logic. Testing only MergeStrict is sufficent.
TestMixedMerge(MiniYaml.MergeStrict(a, b).First().Value);
TestMixedMerge(MiniYaml.MergeStrict(b, a).First().Value);
}
[Ignore("Disabled until the code is fixed so we don't break continuous integration.")]
[TestCase(TestName = "MergeLiberal(MiniYaml, MiniYaml)")]
public void MergeYamlB()
void TestMixedMerge(MiniYaml result)
{
var res = MiniYaml.MergeLiberal(parent, child);
InheritanceTest(res.Nodes);
}
[Ignore("Disabled until the code is fixed so we don't break continuous integration.")]
[TestCase(TestName = "MergeStrict(List<MiniYamlNode>, List<MiniYamlNode>)")]
public void MergeYamlC()
{
var res = MiniYaml.MergeStrict(parentList, childList).Last();
Assert.That(res.Key, Is.EqualTo("Child"));
InheritanceTest(res.Value.Nodes);
}
[Ignore("Disabled until the code is fixed so we don't break continuous integration.")]
[TestCase(TestName = "MergeLiberal(List<MiniYamlNode>, List<MiniYamlNode>)")]
public void MergeYamlD()
{
var res = MiniYaml.MergeLiberal(parentList, childList).Last();
Assert.That(res.Key, Is.EqualTo("Child"));
InheritanceTest(res.Value.Nodes);
Console.WriteLine(result.ToLines("result").JoinWith("\n"));
Assert.That(result.Nodes.Any(n => n.Key == "FromA"), Is.True, "Node from A");
Assert.That(result.Nodes.Any(n => n.Key == "FromB"), Is.True, "Node from B");
Assert.That(result.Nodes.Any(n => n.Key == "FromARemovedA"), Is.Not.True, "Node from A removed by A");
Assert.That(result.Nodes.Any(n => n.Key == "FromARemovedB"), Is.Not.True, "Node from A removed by B");
Assert.That(result.Nodes.Any(n => n.Key == "FromBRemovedA"), Is.Not.True, "Node from B removed by A");
Assert.That(result.Nodes.Any(n => n.Key == "FromBRemovedB"), Is.Not.True, "Node from B removed by B");
}
[TestCase(TestName = "Mixed tabs & spaces indents")]

View File

@@ -1330,7 +1330,7 @@ Rules:
Health:
HP: 200
E7:
-Selectable:
-AnnounceOnKill:
powerproxy.paratroopers:
ParatroopersPower:
DisplayBeacon: false

View File

@@ -2102,7 +2102,7 @@ Rules:
MissionObjectives:
EarlyGameOver: true
World:
-CrateDrop:
-CrateSpawner:
-SpawnMPUnits:
-MPStartLocations:
LuaScript:

View File

@@ -666,7 +666,7 @@ Rules:
ParatroopersPower@paratroopers:
ChargeTime: 60
DropItems: E1,E1,E1,E2,E2
#-RallyPoint: # TODO https://github.com/OpenRA/OpenRA/issues/6818
-RallyPoint:
-Sellable:
DOME:
-Sellable:

View File

@@ -1029,7 +1029,7 @@ Rules:
MissionObjectives:
EarlyGameOver: true
World:
-CrateDrop:
-CrateSpawner:
-SpawnMPUnits:
-MPStartLocations:
LuaScript:

View File

@@ -1,5 +1,40 @@
^Vehicle:
^ExistsInWorld:
AppearsOnRadar:
UpdatesPlayerStatistics:
CombatDebugOverlay:
DrawLineToTarget:
GivesExperience:
BodyOrientation:
ScriptTriggers:
UpgradeManager:
Huntable:
^GainsExperience:
GainsExperience:
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
^IronCurtainable:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
^Vehicle:
Inherits@1: ^ExistsInWorld
Inherits@2: ^GainsExperience
Inherits@3: ^IronCurtainable
Mobile:
Crushes: mine, crate
TerrainSpeeds:
@@ -22,9 +57,6 @@
CargoType: Vehicle
AttackMove:
HiddenUnderFog:
GainsExperience:
GivesExperience:
DrawLineToTarget:
ActorLostNotification:
ProximityCaptor:
Types: Vehicle
@@ -32,11 +64,8 @@
GpsDot:
String: Vehicle
WithSmoke:
UpdatesPlayerStatistics:
CombatDebugOverlay:
Guard:
Guardable:
BodyOrientation:
Tooltip:
GenericName: Vehicle
EjectOnDeath:
@@ -45,36 +74,18 @@
EjectOnGround: true
EjectInAir: false
AllowUnsuitableCell: false
Huntable:
Capturable:
Type: vehicle
CaptureThreshold: 1
CancelActivity: True
CaptureNotification:
Notification: UnitStolen
ScriptTriggers:
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
UpgradeManager:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
MustBeDestroyed:
^Tank:
AppearsOnRadar:
Inherits@1: ^ExistsInWorld
Inherits@2: ^GainsExperience
Inherits@3: ^IronCurtainable
Mobile:
Crushes: wall, mine, crate
TerrainSpeeds:
@@ -97,9 +108,6 @@
CargoType: Vehicle
AttackMove:
HiddenUnderFog:
GainsExperience:
GivesExperience:
DrawLineToTarget:
ActorLostNotification:
ProximityCaptor:
Types: Tank
@@ -107,11 +115,8 @@
GpsDot:
String: Vehicle
WithSmoke:
UpdatesPlayerStatistics:
CombatDebugOverlay:
Guard:
Guardable:
BodyOrientation:
Tooltip:
GenericName: Tank
EjectOnDeath:
@@ -120,32 +125,12 @@
EjectOnGround: true
EjectInAir: false
AllowUnsuitableCell: false
Huntable:
Capturable:
Type: vehicle
CaptureThreshold: 1
CancelActivity: True
CaptureNotification:
Notification: UnitStolen
ScriptTriggers:
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
UpgradeManager:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
MustBeDestroyed:
Parachutable:
ParachuteOffset: 0,0,200
@@ -158,7 +143,8 @@
WaterCorpsePalette:
^Infantry:
AppearsOnRadar:
Inherits@1: ^ExistsInWorld
Inherits@2: ^GainsExperience
Health:
Radius: 128
Armor:
@@ -189,9 +175,6 @@
Passenger:
CargoType: Infantry
HiddenUnderFog:
GainsExperience:
GivesExperience:
DrawLineToTarget:
ActorLostNotification:
ProximityCaptor:
Types: Infantry
@@ -200,11 +183,8 @@
String: Infantry
Crushable:
CrushSound: squishy2.aud
UpdatesPlayerStatistics:
CombatDebugOverlay:
Guard:
Guardable:
BodyOrientation:
Tooltip:
GenericName: Soldier
SelfHealing@HOSPITAL:
@@ -217,8 +197,6 @@
GlobalUpgradable:
Upgrades: hospitalheal
Prerequisites: hosp
Huntable:
ScriptTriggers:
DeathSounds@NORMAL:
DeathTypes: 1, 2, 3, 4
DeathSounds@BURNED:
@@ -236,19 +214,12 @@
WaterImpactSound: splash9.aud
Cloneable:
Types: Infantry
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
UpgradeManager:
MustBeDestroyed:
^Ship:
AppearsOnRadar:
Inherits@1: ^ExistsInWorld
Inherits@2: ^GainsExperience
Inherits@3: ^IronCurtainable
Mobile:
Crushes: crate
TerrainSpeeds:
@@ -260,9 +231,6 @@
TargetTypes: Ground, Water, Repair
HiddenUnderFog:
AttackMove:
GainsExperience:
GivesExperience:
DrawLineToTarget:
ActorLostNotification:
Notification: NavalUnitLost
ProximityCaptor:
@@ -271,37 +239,16 @@
GpsDot:
String: Ship
WithSmoke:
UpdatesPlayerStatistics:
CombatDebugOverlay:
Guard:
Guardable:
BodyOrientation:
Tooltip:
GenericName: Ship
Huntable:
ScriptTriggers:
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
UpgradeManager:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
UpgradeMinEnabledLevel: 1
MustBeDestroyed:
^Plane:
Inherits@1: ^ExistsInWorld
Inherits@2: ^GainsExperience
Inherits@3: ^IronCurtainable
AppearsOnRadar:
UseLocation: true
SelectionDecorations:
@@ -314,9 +261,6 @@
AttackMove:
Guard:
Guardable:
GainsExperience:
GivesExperience:
DrawLineToTarget:
ActorLostNotification:
Notification: AirUnitLost
ProximityCaptor:
@@ -330,31 +274,8 @@
GivesBounty:
GpsDot:
String: Plane
UpdatesPlayerStatistics:
CombatDebugOverlay:
BodyOrientation:
Tooltip:
GenericName: Plane
Huntable:
ScriptTriggers:
GainsStatUpgrades:
SelfHealing@ELITE:
Step: 2
Ticks: 100
HealIfBelow: 1
DamageCooldown: 125
UpgradeTypes: selfheal
UpgradeMinEnabledLevel: 1
UpgradeManager:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
WithShadow:
MustBeDestroyed:
@@ -367,7 +288,8 @@
Hovers:
^Building:
AppearsOnRadar:
Inherits@1: ^ExistsInWorld
Inherits@2: ^IronCurtainable
SelectionDecorations:
Selectable:
Priority: 3
@@ -393,7 +315,6 @@
ActorTypes: e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,e1,c1,c2,c3,c4,c5,c6,c7,c8,c9,c10,e6,e6,e6,e6,e6
MustBeDestroyed:
RequiredForShortGame: true
GivesExperience:
CaptureNotification:
EditorAppearance:
RelativeToTopLeft: true
@@ -404,29 +325,14 @@
SellSounds: cashturn.aud
AcceptsSupplies:
GivesBounty:
UpdatesPlayerStatistics:
CombatDebugOverlay:
Guardable:
Range: 3
BodyOrientation:
FrozenUnderFog:
Tooltip:
GenericName: Structure
GpsDot:
String: Structure
Huntable:
Demolishable:
ScriptTriggers:
UpgradeManager:
UpgradeOverlay@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
InvulnerabilityUpgrade@IRONCURTAIN:
UpgradeTypes: invulnerability
UpgradeMinEnabledLevel: 1
UpgradeMaxAcceptedLevel: 2
TimedUpgradeBar:
Upgrade: invulnerability
^Defense:
Inherits: ^Building