From 17daac11a114fa9ca2fa3fbe8677aaad0268efc3 Mon Sep 17 00:00:00 2001 From: abcdefg30 Date: Thu, 21 Jan 2016 16:18:14 +0100 Subject: [PATCH 1/4] Avoid unnecessary lookups in the production logic by improving the GetBuildTime method and moving lookups around. --- .../Traits/Player/ClassicProductionQueue.cs | 9 +++++++-- OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs | 12 ++++++++---- .../Widgets/Logic/Ingame/ProductionTooltipLogic.cs | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs index 876e238474..3e8d2c4aef 100644 --- a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs @@ -125,14 +125,19 @@ namespace OpenRA.Mods.Common.Traits if (bi == null) return 0; + return GetBuildTime(ai); + } + + public override int GetBuildTime(ActorInfo unit) + { if (self.World.AllowDevCommands && self.Owner.PlayerActor.Trait().FastBuild) return 0; - var time = (int)(ai.GetBuildTime() * Info.BuildSpeed); + var time = (int)(unit.GetBuildTime() * Info.BuildSpeed); if (info.SpeedUp) { - var type = bi.BuildAtProductionType ?? info.Type; + var type = unit.TraitInfo().BuildAtProductionType ?? info.Type; var selfsameProductionsCount = self.World.ActorsWithTrait() .Count(p => p.Actor.Owner == self.Owner && p.Trait.Info.Produces.Contains(type)); diff --git a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs index c0c720bf43..7c4a6678f6 100644 --- a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs @@ -256,9 +256,6 @@ namespace OpenRA.Mods.Common.Traits if (!bi.Queue.Contains(Info.Type)) return; - var cost = unit.HasTraitInfo() ? unit.TraitInfo().Cost : 0; - var time = GetBuildTime(order.TargetString); - // You can't build that if (BuildableItems().All(b => b.Name != order.TargetString)) return; @@ -275,6 +272,9 @@ namespace OpenRA.Mods.Common.Traits return; } + var valued = unit.TraitInfoOrDefault(); + var cost = valued != null ? valued.Cost : 0; + var time = GetBuildTime(unit); var amountToBuild = Math.Min(fromLimit, order.ExtraData); for (var n = 0; n < amountToBuild; n++) { @@ -313,11 +313,15 @@ namespace OpenRA.Mods.Common.Traits if (unit == null || !unit.HasTraitInfo()) return 0; + return GetBuildTime(unit); + } + + public virtual int GetBuildTime(ActorInfo unit) + { if (self.World.AllowDevCommands && self.Owner.PlayerActor.Trait().FastBuild) return 0; var time = unit.GetBuildTime() * Info.BuildSpeed; - return (int)time; } diff --git a/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs b/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs index a9afe10719..545c63ea14 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs @@ -81,7 +81,7 @@ namespace OpenRA.Mods.Common.Widgets.Logic powerIcon.IsVisible = () => power != 0; var lowpower = pm.PowerState != PowerState.Normal; - var time = palette.CurrentQueue == null ? 0 : palette.CurrentQueue.GetBuildTime(actor.Name) + var time = palette.CurrentQueue == null ? 0 : palette.CurrentQueue.GetBuildTime(actor) * (lowpower ? palette.CurrentQueue.Info.LowPowerSlowdown : 1); var timeString = WidgetUtils.FormatTime(time, world.Timestep); timeLabel.GetText = () => timeString; From 5496245a00e548898bbb9f411597503b6a28dc31 Mon Sep 17 00:00:00 2001 From: abcdefg30 Date: Thu, 21 Jan 2016 16:53:46 +0100 Subject: [PATCH 2/4] Remove unnecessary lookups and checks - We already return early if the unit doesn't have a BuildableInfo - World.Map.Rules.Actors[actorName] won't return null - Made BuildUnit use an ActorInfo instead of the name as parameter --- .../Traits/Player/ClassicProductionQueue.cs | 18 ++++++---------- .../Traits/Player/ProductionQueue.cs | 21 +++++++++---------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs index 3e8d2c4aef..051727eb5f 100644 --- a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs @@ -85,14 +85,13 @@ namespace OpenRA.Mods.Common.Traits .FirstOrDefault(); } - protected override bool BuildUnit(string name) + protected override bool BuildUnit(ActorInfo unit) { // Find a production structure to build this actor - var ai = self.World.Map.Rules.Actors[name]; - var bi = ai.TraitInfoOrDefault(); + var bi = unit.TraitInfo(); // Some units may request a specific production type, which is ignored if the AllTech cheat is enabled - var type = bi == null || developerMode.AllTech ? Info.Type : bi.BuildAtProductionType ?? Info.Type; + var type = developerMode.AllTech ? Info.Type : bi.BuildAtProductionType ?? Info.Type; var producers = self.World.ActorsWithTrait() .Where(x => x.Actor.Owner == self.Owner @@ -102,13 +101,13 @@ namespace OpenRA.Mods.Common.Traits if (!producers.Any()) { - CancelProduction(name, 1); + CancelProduction(unit.Name, 1); return true; } foreach (var p in producers.Where(p => !p.Actor.IsDisabled())) { - if (p.Trait.Produce(p.Actor, ai, p.Trait.Faction)) + if (p.Trait.Produce(p.Actor, unit, p.Trait.Faction)) { FinishProduction(); return true; @@ -120,12 +119,7 @@ namespace OpenRA.Mods.Common.Traits public override int GetBuildTime(string unitString) { - var ai = self.World.Map.Rules.Actors[unitString]; - var bi = ai.TraitInfoOrDefault(); - if (bi == null) - return 0; - - return GetBuildTime(ai); + return GetBuildTime(self.World.Map.Rules.Actors[unitString]); } public override int GetBuildTime(ActorInfo unit) diff --git a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs index 7c4a6678f6..fd0ccbc421 100644 --- a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs @@ -232,7 +232,8 @@ namespace OpenRA.Mods.Common.Traits { while (queue.Count > 0 && BuildableItems().All(b => b.Name != queue[0].Item)) { - playerResources.GiveCash(queue[0].TotalCost - queue[0].RemainingCost); // refund what's been paid so far. + // Refund what's been paid so far + playerResources.GiveCash(queue[0].TotalCost - queue[0].RemainingCost); FinishProduction(); } @@ -287,7 +288,7 @@ namespace OpenRA.Mods.Common.Traits hasPlayedSound = Game.Sound.PlayNotification(rules, self.Owner, "Speech", Info.ReadyAudio, self.Owner.Faction.InternalName); else if (!isBuilding) { - if (BuildUnit(order.TargetString)) + if (BuildUnit(unit)) Game.Sound.PlayNotification(rules, self.Owner, "Speech", Info.ReadyAudio, self.Owner.Faction.InternalName); else if (!hasPlayedSound && time > 0) hasPlayedSound = Game.Sound.PlayNotification(rules, self.Owner, "Speech", Info.BlockedAudio, self.Owner.Faction.InternalName); @@ -309,11 +310,7 @@ namespace OpenRA.Mods.Common.Traits public virtual int GetBuildTime(string unitString) { - var unit = self.World.Map.Rules.Actors[unitString]; - if (unit == null || !unit.HasTraitInfo()) - return 0; - - return GetBuildTime(unit); + return GetBuildTime(self.World.Map.Rules.Actors[unitString]); } public virtual int GetBuildTime(ActorInfo unit) @@ -340,7 +337,9 @@ namespace OpenRA.Mods.Common.Traits else if (lastIndex == 0) { var item = queue[0]; - playerResources.GiveCash(item.TotalCost - item.RemainingCost); // refund what has been paid + + // Refund what has been paid + playerResources.GiveCash(item.TotalCost - item.RemainingCost); FinishProduction(); } } @@ -365,17 +364,17 @@ namespace OpenRA.Mods.Common.Traits // Builds a unit from the actor that holds this queue (1 queue per building) // Returns false if the unit can't be built - protected virtual bool BuildUnit(string name) + protected virtual bool BuildUnit(ActorInfo unit) { // Cannot produce if I'm dead if (!self.IsInWorld || self.IsDead) { - CancelProduction(name, 1); + CancelProduction(unit.Name, 1); return true; } var sp = self.TraitsImplementing().FirstOrDefault(p => p.Info.Produces.Contains(Info.Type)); - if (sp != null && !self.IsDisabled() && sp.Produce(self, self.World.Map.Rules.Actors[name], Faction)) + if (sp != null && !self.IsDisabled() && sp.Produce(self, unit, Faction)) { FinishProduction(); return true; From 4e0775b59e677f01ca5bc5a7297000b6b755651d Mon Sep 17 00:00:00 2001 From: abcdefg30 Date: Thu, 21 Jan 2016 17:04:13 +0100 Subject: [PATCH 3/4] Remove the unused ISync implementation in ClassicProductionQueue.cs The base class ProductionQueue already implements ISync --- OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs index 051727eb5f..718f34caf3 100644 --- a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs @@ -30,7 +30,7 @@ namespace OpenRA.Mods.Common.Traits public override object Create(ActorInitializer init) { return new ClassicProductionQueue(init, this); } } - public class ClassicProductionQueue : ProductionQueue, ISync + public class ClassicProductionQueue : ProductionQueue { static readonly ActorInfo[] NoItems = { }; From ed3f3706f98a9762f2b2dc1249173975bb8b9acd Mon Sep 17 00:00:00 2001 From: abcdefg30 Date: Thu, 21 Jan 2016 22:29:01 +0100 Subject: [PATCH 4/4] Reduce trait lookups further by adding a BuildableInfo parameter to GetBuildTime --- OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs | 4 ++-- OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs | 4 ++-- .../Widgets/Logic/Ingame/ProductionTooltipLogic.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs index 718f34caf3..7b181633a1 100644 --- a/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ClassicProductionQueue.cs @@ -122,7 +122,7 @@ namespace OpenRA.Mods.Common.Traits return GetBuildTime(self.World.Map.Rules.Actors[unitString]); } - public override int GetBuildTime(ActorInfo unit) + public override int GetBuildTime(ActorInfo unit, BuildableInfo bi = null) { if (self.World.AllowDevCommands && self.Owner.PlayerActor.Trait().FastBuild) return 0; @@ -131,7 +131,7 @@ namespace OpenRA.Mods.Common.Traits if (info.SpeedUp) { - var type = unit.TraitInfo().BuildAtProductionType ?? info.Type; + var type = (bi ?? unit.TraitInfo()).BuildAtProductionType ?? info.Type; var selfsameProductionsCount = self.World.ActorsWithTrait() .Count(p => p.Actor.Owner == self.Owner && p.Trait.Info.Produces.Contains(type)); diff --git a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs index fd0ccbc421..5f09e283d5 100644 --- a/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs +++ b/OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs @@ -275,7 +275,7 @@ namespace OpenRA.Mods.Common.Traits var valued = unit.TraitInfoOrDefault(); var cost = valued != null ? valued.Cost : 0; - var time = GetBuildTime(unit); + var time = GetBuildTime(unit, bi); var amountToBuild = Math.Min(fromLimit, order.ExtraData); for (var n = 0; n < amountToBuild; n++) { @@ -313,7 +313,7 @@ namespace OpenRA.Mods.Common.Traits return GetBuildTime(self.World.Map.Rules.Actors[unitString]); } - public virtual int GetBuildTime(ActorInfo unit) + public virtual int GetBuildTime(ActorInfo unit, BuildableInfo bi = null) { if (self.World.AllowDevCommands && self.Owner.PlayerActor.Trait().FastBuild) return 0; diff --git a/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs b/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs index 545c63ea14..95153af7fc 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Ingame/ProductionTooltipLogic.cs @@ -81,7 +81,7 @@ namespace OpenRA.Mods.Common.Widgets.Logic powerIcon.IsVisible = () => power != 0; var lowpower = pm.PowerState != PowerState.Normal; - var time = palette.CurrentQueue == null ? 0 : palette.CurrentQueue.GetBuildTime(actor) + var time = palette.CurrentQueue == null ? 0 : palette.CurrentQueue.GetBuildTime(actor, buildable) * (lowpower ? palette.CurrentQueue.Info.LowPowerSlowdown : 1); var timeString = WidgetUtils.FormatTime(time, world.Timestep); timeLabel.GetText = () => timeString;