From 611a928a470621a7f4c15704fde541d1585e66df Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Tue, 22 Dec 2015 16:28:40 +0000 Subject: [PATCH 1/2] Prevent changing world players after being set once. --- OpenRA.Game/World.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OpenRA.Game/World.cs b/OpenRA.Game/World.cs index 4cacd3392d..708e253c40 100644 --- a/OpenRA.Game/World.cs +++ b/OpenRA.Game/World.cs @@ -42,6 +42,8 @@ namespace OpenRA public void SetPlayers(IEnumerable players, Player localPlayer) { + if (Players.Length > 0) + throw new InvalidOperationException("Players are fixed once they have been set."); Players = players.ToArray(); SetLocalPlayer(localPlayer); } From 3a2139de269d2ade3cdac97e084cab89966306c0 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Tue, 22 Dec 2015 23:15:32 +0000 Subject: [PATCH 2/2] Add PlayerDictionary. This custom collection allows other classes to implement a Player to value mapping, but also stores the values in an array for faster lookup by the player index in the world. For some code, this improved lookup time is important for performance. --- OpenRA.Game/OpenRA.Game.csproj | 1 + OpenRA.Game/Primitives/PlayerDictionary.cs | 49 +++++++++++++++++++ .../Traits/Modifiers/FrozenUnderFog.cs | 38 ++++++-------- OpenRA.Mods.RA/Effects/GpsDot.cs | 15 +++--- 4 files changed, 73 insertions(+), 30 deletions(-) create mode 100644 OpenRA.Game/Primitives/PlayerDictionary.cs diff --git a/OpenRA.Game/OpenRA.Game.csproj b/OpenRA.Game/OpenRA.Game.csproj index 7c2b509a44..b58b5607f4 100644 --- a/OpenRA.Game/OpenRA.Game.csproj +++ b/OpenRA.Game/OpenRA.Game.csproj @@ -153,6 +153,7 @@ + diff --git a/OpenRA.Game/Primitives/PlayerDictionary.cs b/OpenRA.Game/Primitives/PlayerDictionary.cs new file mode 100644 index 0000000000..2d2d317aa9 --- /dev/null +++ b/OpenRA.Game/Primitives/PlayerDictionary.cs @@ -0,0 +1,49 @@ +using System; +using System.Collections.Generic; + +namespace OpenRA.Primitives +{ + /// + /// Provides a mapping of players to values, as well as fast lookup by player index. + /// + public struct PlayerDictionary : IReadOnlyList, IReadOnlyDictionary where T : class + { + readonly T[] valueByPlayerIndex; + readonly Dictionary valueByPlayer; + + public PlayerDictionary(World world, Func valueFactory) + { + var players = world.Players; + if (players.Length == 0) + throw new InvalidOperationException("The players in the world have not yet been set."); + + // The world players never change, so we can safely maintain an array of values. + // We need to enforce T is a class, so if it changes that change is available in both collections. + valueByPlayerIndex = new T[players.Length]; + valueByPlayer = new Dictionary(players.Length); + for (var i = 0; i < players.Length; i++) + { + var player = players[i]; + var state = valueFactory(player); + valueByPlayerIndex[i] = state; + valueByPlayer[player] = state; + } + } + + /// Gets the value for the specified player. This is slower than specifying a player index. + public T this[Player player] { get { return valueByPlayer[player]; } } + + /// Gets the value for the specified player index. + public T this[int playerIndex] { get { return valueByPlayerIndex[playerIndex]; } } + + public int Count { get { return valueByPlayerIndex.Length; } } + public IEnumerator GetEnumerator() { return ((IEnumerable)valueByPlayerIndex).GetEnumerator(); } + + ICollection IReadOnlyDictionary.Keys { get { return valueByPlayer.Keys; } } + ICollection IReadOnlyDictionary.Values { get { return valueByPlayer.Values; } } + bool IReadOnlyDictionary.ContainsKey(Player key) { return valueByPlayer.ContainsKey(key); } + bool IReadOnlyDictionary.TryGetValue(Player key, out T value) { return valueByPlayer.TryGetValue(key, out value); } + IEnumerator> IEnumerable>.GetEnumerator() { return valueByPlayer.GetEnumerator(); } + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { return GetEnumerator(); } + } +} diff --git a/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs b/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs index 1ef4d701d6..830bb60c6d 100644 --- a/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs +++ b/OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs @@ -12,6 +12,7 @@ using System; using System.Collections.Generic; using System.Linq; using OpenRA.Graphics; +using OpenRA.Primitives; using OpenRA.Traits; namespace OpenRA.Mods.Common.Traits @@ -35,9 +36,7 @@ namespace OpenRA.Mods.Common.Traits readonly bool startsRevealed; readonly PPos[] footprint; - readonly Dictionary stateByPlayer = new Dictionary(); - - FrozenState[] stateByPlayerIndex; + PlayerDictionary frozenStates; ITooltip tooltip; Health health; bool initialized; @@ -71,7 +70,7 @@ namespace OpenRA.Mods.Common.Traits if (!byPlayer.Shroud.FogEnabled) return byPlayer.Shroud.AnyExplored(self.OccupiesSpace.OccupiedCells()); - return initialized && stateByPlayer[byPlayer].IsVisible; + return initialized && frozenStates[byPlayer].IsVisible; } public bool IsVisible(Actor self, Player byPlayer) @@ -89,41 +88,36 @@ namespace OpenRA.Mods.Common.Traits return; VisibilityHash = 0; - var players = self.World.Players; if (!initialized) { - // The world players never change, so we can safely index this collection. - stateByPlayerIndex = new FrozenState[players.Length]; + frozenStates = new PlayerDictionary(self.World, player => + { + var frozenActor = new FrozenActor(self, footprint, player.Shroud, startsRevealed); + player.PlayerActor.Trait().Add(frozenActor); + return new FrozenState(frozenActor) { IsVisible = startsRevealed }; + }); tooltip = self.TraitsImplementing().FirstOrDefault(); health = self.TraitOrDefault(); } - for (var i = 0; i < players.Length; i++) + for (var playerIndex = 0; playerIndex < frozenStates.Count; playerIndex++) { - FrozenActor frozenActor; + var state = frozenStates[playerIndex]; + var frozenActor = state.FrozenActor; bool isVisible; if (!initialized) { - var player = players[i]; - frozenActor = new FrozenActor(self, footprint, player.Shroud, startsRevealed); - isVisible = startsRevealed; - var state = new FrozenState(frozenActor) { IsVisible = isVisible }; - stateByPlayer.Add(player, state); - stateByPlayerIndex[i] = state; - player.PlayerActor.Trait().Add(frozenActor); + isVisible = state.IsVisible; } else { - // PERF: Minimize lookup cost by combining all state into one, and using an array rather than a dictionary. - var state = stateByPlayerIndex[i]; - frozenActor = state.FrozenActor; isVisible = !frozenActor.Visible; state.IsVisible = isVisible; } if (isVisible) - VisibilityHash |= 1 << (i % 32); + VisibilityHash |= 1 << (playerIndex % 32); else continue; @@ -151,9 +145,9 @@ namespace OpenRA.Mods.Common.Traits return; IRenderable[] renderables = null; - foreach (var player in self.World.Players) + for (var playerIndex = 0; playerIndex < frozenStates.Count; playerIndex++) { - var frozen = stateByPlayer[player].FrozenActor; + var frozen = frozenStates[playerIndex].FrozenActor; if (!frozen.NeedRenderables) continue; diff --git a/OpenRA.Mods.RA/Effects/GpsDot.cs b/OpenRA.Mods.RA/Effects/GpsDot.cs index 1ff8497ef9..562d68380e 100644 --- a/OpenRA.Mods.RA/Effects/GpsDot.cs +++ b/OpenRA.Mods.RA/Effects/GpsDot.cs @@ -37,7 +37,7 @@ namespace OpenRA.Mods.RA.Effects readonly GpsDotInfo info; readonly Animation anim; - readonly Dictionary stateByPlayer = new Dictionary(); + readonly PlayerDictionary dotStates; readonly Lazy huf; readonly Lazy fuf; readonly Lazy disguise; @@ -70,13 +70,12 @@ namespace OpenRA.Mods.RA.Effects cloak = Exts.Lazy(() => self.TraitOrDefault()); frozen = new Cache(p => p.PlayerActor.Trait()); - - stateByPlayer = self.World.Players.ToDictionary(p => p, p => new DotState(p.PlayerActor.Trait())); + dotStates = new PlayerDictionary(self.World, player => new DotState(player.PlayerActor.Trait())); } public bool IsDotVisible(Player toPlayer) { - return stateByPlayer[toPlayer].IsTargetable; + return dotStates[toPlayer].IsTargetable; } bool IsTargetableBy(Player toPlayer, out bool shouldRenderIndicator) @@ -122,11 +121,11 @@ namespace OpenRA.Mods.RA.Effects if (!self.IsInWorld || self.IsDead) return; - foreach (var player in self.World.Players) + for (var playerIndex = 0; playerIndex < dotStates.Count; playerIndex++) { - var state = stateByPlayer[player]; + var state = dotStates[playerIndex]; var shouldRender = false; - var targetable = (state.Gps.Granted || state.Gps.GrantedAllies) && IsTargetableBy(player, out shouldRender); + var targetable = (state.Gps.Granted || state.Gps.GrantedAllies) && IsTargetableBy(world.Players[playerIndex], out shouldRender); state.IsTargetable = targetable; state.ShouldRender = targetable && shouldRender; } @@ -134,7 +133,7 @@ namespace OpenRA.Mods.RA.Effects public IEnumerable Render(WorldRenderer wr) { - if (self.World.RenderPlayer == null || !stateByPlayer[self.World.RenderPlayer].ShouldRender || self.Disposed) + if (self.World.RenderPlayer == null || !dotStates[self.World.RenderPlayer].ShouldRender || self.Disposed) return SpriteRenderable.None; var palette = wr.Palette(info.IndicatorPalettePrefix + self.Owner.InternalName);