Rename the Radar Dome in Soviets08a to avoid a crash#21251
Conversation
abcdefg30
force-pushed
the
sovs08aRadar
branch
from
b821ec0 to
acb0f75
Compare
|
Added a lint check. Seems like Soviets08a was the only mission to run into this issue. |
RoosterDragon
left a comment
RoosterDragon
left a comment
There was a problem hiding this comment.
Some suggestions since you'll need to squash for the style error either way.
| foreach (var actor in actorTypes) | ||
| if (!map.Rules.Actors.Keys.Contains(actor.ToLowerInvariant())) | ||
| emitError($"Actor `{actor}` is not defined by any rule."); | ||
| var scriptBindings = Game.ModData.ObjectCreator.GetTypesImplementing<ScriptGlobal>().SelectMany(t => Utility.GetCustomAttributes<ScriptGlobalAttribute>(t, true)).ToArray(); |
There was a problem hiding this comment.
Consider .Select(x => x.Name).ToHashSet() for fast Contains check inside the loop. (I'm not sure if there's many bindings but just in case)
| foreach (var actor in map.ActorDefinitions) | ||
| { | ||
| var name = actor.Value.Value; | ||
| if (!map.Rules.Actors.Keys.Contains(name.ToLowerInvariant())) |
There was a problem hiding this comment.
Should this be ContainsKey? (previous code but since we're here)
There was a problem hiding this comment.
Is there a difference? Will change to ContainsKey regardless since it feels more natural.
There was a problem hiding this comment.
ContainsKey does a fast lookup in the dictionary. Keys.Contains does a slow linear search over the keys collection.
… names Only scripted maps will have the need to use named actors, so we can assume that there will be a Lua script used in maps with such actors.
abcdefg30
force-pushed
the
sovs08aRadar
branch
from
5b4bd18 to
500ffe8
Compare
|
Updated. |
3 participants
Closes #21250. The issue is that the name
Radarconflicts with theOpenRA/OpenRA.Mods.Common/Scripting/Global/RadarGlobal.cs
Line 18 in dd7441e