Fix an invalid cast exception in the Ship interdiction event. Reduc…

…e reliance on `Superpower.None` and `Government.None` when a null value is more appropriate.
EDCD · Dec 7, 2023 · 84679da · 84679da
1 parent a19e1c3
commit 84679da
Show file tree

Hide file tree

Showing 8 changed files with 53 additions and 18 deletions.
diff --git a/CrimeMonitor/ConfigurationWindow.xaml.cs b/CrimeMonitor/ConfigurationWindow.xaml.cs
@@ -57,7 +57,7 @@ private void updateRecord(object sender, RoutedEventArgs e)
 
                 Thread factionStationThread = new Thread(() =>
                 {
-                    Superpower Allegiance = Superpower.FromNameOrEdName(record.faction);
+                    var Allegiance = Superpower.FromNameOrEdName(record.faction);
                     if (Allegiance == null)
                     {
                         crimeMonitor()?.GetFactionData(record, record.system);

diff --git a/CrimeMonitor/CrimeMonitor.cs b/CrimeMonitor/CrimeMonitor.cs
@@ -853,7 +853,7 @@ private FactionRecord AddRecord(string faction)
             if (faction == null) { return null; }
 
             FactionRecord record = new FactionRecord(faction);
-            Superpower Allegiance = Superpower.FromNameOrEdName(faction);
+            var Allegiance = Superpower.FromNameOrEdName(faction);
             if (Allegiance == null)
             {
                 GetFactionData(record);
@@ -1015,7 +1015,7 @@ public void GetFactionData(FactionRecord record, string homeSystem = null)
 
             // Get the faction from Elite BGS and set faction record values
             Faction faction = bgsService.GetFactionByName(record.faction);
-            record.Allegiance = faction.Allegiance ?? Superpower.None;
+            record.Allegiance = faction.Allegiance;
 
             // Check faction with archived home systems
             if (homeSystems.TryGetValue(record.faction, out string result))
@@ -1123,7 +1123,7 @@ public void UpdateStations()
                 {
                     foreach (FactionRecord record in criminalrecord.ToList())
                     {
-                        Superpower Allegiance = Superpower.FromNameOrEdName(record.faction);
+                        var Allegiance = Superpower.FromNameOrEdName(record.faction);
                         if (Allegiance == null)
                         {
                             record.station = GetFactionStation(record.system);

diff --git a/DataDefinitions/Faction.cs b/DataDefinitions/Faction.cs
@@ -39,10 +39,10 @@ public class Faction
         public long? EDSMID { get; set; }
 
         /// <summary> The faction's allegiance </summary>
-        public Superpower Allegiance { get; set; } = Superpower.None;
+        public Superpower Allegiance { get; set; }
 
         /// <summary> The faction's government </summary>
-        public Government Government { get; set; } = Government.None;
+        public Government Government { get; set; }
 
         /// <summary> Whether the faction is a player faction </summary>
         public bool? isplayer { get; set; }

diff --git a/DataDefinitions/Superpower.cs b/DataDefinitions/Superpower.cs
@@ -41,12 +41,12 @@ public Superpower() : this("")
 
         public static Superpower FromNameOrEdName(string from)
         {
-            if (from == null)
+            if (string.IsNullOrEmpty(from))
             {
-                return None;
+                return null;
             }
 
-            Superpower result = FromName(from);
+            var result = FromName(from);
             if (result == null && from.StartsWith("$faction_"))
             {
                 result = FromEDName(from);

diff --git a/JournalMonitor/JournalMonitor.cs b/JournalMonitor/JournalMonitor.cs
@@ -1835,7 +1835,7 @@ async void ModuleArrived()
                                     string interdictee = JsonParsing.getString(data, "Interdicted");
                                     bool iscommander = JsonParsing.getBool(data, "IsPlayer");
                                     data.TryGetValue("CombatRank", out object val);
-                                    CombatRating rating = (val == null ? null : CombatRating.FromRank((int)val));
+                                    var rating = ( val == null ? null : CombatRating.FromRank( Convert.ToInt32( val ) ) );
                                     string faction = GetFactionName(data, "Faction");
                                     string power = JsonParsing.getString(data, "Power");
 
@@ -5071,9 +5071,8 @@ private static void GetPowerplayData(IDictionary<string, object> data, out List<
         private static Superpower GetAllegiance(IDictionary<string, object> data, string key)
         {
             data.TryGetValue(key, out object val);
-            // FD sends "" rather than null; fix that here
-            if (((string)val) == "") { val = null; }
-            return Superpower.FromNameOrEdName((string)val);
+            // FD may send "" rather than null;
+            return Superpower.FromNameOrEdName(Convert.ToString(val));
         }
 
         private static List<Conflict> GetConflicts(object conflictsVal, List<Faction> factions)

diff --git a/SpeechResponder/CustomFunctions/SuperpowerDetails.cs b/SpeechResponder/CustomFunctions/SuperpowerDetails.cs
@@ -16,7 +16,7 @@ public class SuperpowerDetails : ICustomFunction
         public Type ReturnType => typeof( Superpower );
         public NativeFunction function => new NativeFunction((values) =>
         {
-            var result = Superpower.FromNameOrEdName(values[0].AsString);
+            var result = Superpower.FromNameOrEdName(values[0].AsString) ?? Superpower.None;
             return new ReflectionValue(result ?? new object());
         }, 1);
     }

diff --git a/Tests/BgsDataTests.cs b/Tests/BgsDataTests.cs
@@ -135,8 +135,8 @@ public void TestBgsFactionFromName()
             Faction faction2 = fakeBgsService.GetFactionByName("No such faction");
             Assert.IsNotNull(faction2);
             Assert.AreEqual("No such faction", faction2.name);
-            Assert.AreEqual(Government.None, faction2.Government);
-            Assert.AreEqual(Superpower.None, faction2.Allegiance);
+            Assert.IsNull(faction2.Government);
+            Assert.IsNull(faction2.Allegiance);
             Assert.AreEqual(DateTime.MinValue, faction2.updatedAt);
             Assert.AreEqual(0, faction2.presences.Count);
         }

diff --git a/Tests/JournalMonitorTests.cs b/Tests/JournalMonitorTests.cs
@@ -339,6 +339,40 @@ public void TestJournalInterdiction1()
             string line = @"{ ""timestamp"":""2016-09-21T07:00:17Z"",""event"":""Interdiction"",""Success"":true,""Interdicted"":""Torval's Shield"",""IsPlayer"":false,""Faction"":""Zemina Torval"",""Power"":""Empire""}";
             List<Event> events = JournalMonitor.ParseJournalEntry(line);
             Assert.IsTrue(events.Count == 1);
+            if ( events[0] is ShipInterdictionEvent @event )
+            {
+                Assert.AreEqual( "Zemina Torval", @event.faction );
+                Assert.AreEqual( "Torval's Shield", @event.interdictee );
+                Assert.AreEqual( "Empire", @event.power );
+                Assert.IsNull( @event.rating );
+                Assert.IsTrue( @event.succeeded );
+                Assert.IsFalse( @event.iscommander );
+            }
+            else
+            {
+                Assert.Fail();
+            }
+        }
+
+        [TestMethod]
+        public void TestJournalInterdiction2 ()
+        {
+            string line = @"{ ""timestamp"":""2023-11-29T22:21:30Z"",""event"":""Interdiction"",""Success"":true,""IsPlayer"":true,""Interdicted"":""*redacted*"",""CombatRank"":5}";
+            List<Event> events = JournalMonitor.ParseJournalEntry(line);
+            Assert.IsTrue( events.Count == 1 );
+            if ( events[ 0 ] is ShipInterdictionEvent @event )
+            {
+                Assert.IsNull( @event.faction );
+                Assert.AreEqual( "*redacted*", @event.interdictee );
+                Assert.IsNull( @event.power );
+                Assert.AreEqual(CombatRating.Master.localizedName, @event.rating );
+                Assert.IsTrue( @event.succeeded );
+                Assert.IsTrue( @event.iscommander );
+            }
+            else
+            {
+                Assert.Fail();
+            }
         }
 
         [TestMethod]
@@ -1158,8 +1192,10 @@ public void TestLocationAllegiance()
             LocationEvent @event = (LocationEvent)events[0];
 
             Assert.IsNotNull(@event.raw);
-            Assert.AreEqual("None", @event.controllingsystemfaction.Allegiance?.invariantName);
-            Assert.AreEqual("$faction_None", @event.controllingsystemfaction.Allegiance?.edname);
+            Assert.IsNull(@event.controllingsystemfaction.Allegiance?.invariantName);
+#pragma warning disable CS0618 // Type or member is obsolete
+            Assert.AreEqual( "None", @event.controllingsystemfaction.allegiance);
+#pragma warning restore CS0618 // Type or member is obsolete
         }
 
         [TestMethod]