Fix PersistenceLocationAnalyzer and UniquePlacedNpcAnalyzerissues

Antigen.Skyrim.Tests/ContextualRecords/Placed/Npcs/UniquePlacedNpcAnalyzerTest.cs

@@ -0,0 +1,81 @@
+using Antigen.Skyrim.Record.Placed.Npc;
+using Antigen.Testing.Frameworks;
+using Mutagen.Bethesda.Skyrim;
+using Mutagen.Bethesda.Testing.AutoData;
+using Xunit;
+
+namespace Antigen.Skyrim.Tests.ContextualRecords.Placed.Npcs;
+
+using Fixture = ContextualRecordTestFixture<UniquePlacedNpcAnalyzer, PlacedNpc, IPlacedNpcGetter>;
+
+public class UniquePlacedNpcAnalyzerTest
+{
+    Npc Setup(PlacedNpc rec, ISkyrimMod mod)
+    {
+        var npc = mod.Npcs.AddNew();
+        npc.Configuration.Flags |= NpcConfiguration.Flag.Unique;
+        rec.Base.SetTo(npc);
+        return npc;
+    }
+
+    [Theory, MutagenModAutoData]
+    public void NoPersistLoc(Fixture fixture)
+    {
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                Setup(rec, mod);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                rec.PersistentLocation.SetTo(FormKeys.SkyrimSE.Skyrim.Location.RiverwoodLocation);
+            },
+            UniquePlacedNpcAnalyzer.UniqueNpcWithoutPersistenceLocation);
+    }
+
+    [Theory, MutagenModAutoData]
+    public void NotUnique(Fixture fixture)
+    {
+        Npc? npc = null;
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                npc = Setup(rec, mod);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                npc!.Configuration.Flags &= ~NpcConfiguration.Flag.Unique;
+            },
+            UniquePlacedNpcAnalyzer.UniqueNpcWithoutPersistenceLocation);
+    }
+
+    [Theory, MutagenModAutoData]
+    public void StartDead(Fixture fixture)
+    {
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                Setup(rec, mod);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                rec.MajorFlags |= PlacedNpc.MajorFlag.StartsDead;
+            },
+            UniquePlacedNpcAnalyzer.UniqueNpcWithoutPersistenceLocation);
+    }
+
+    [Theory, MutagenModAutoData]
+    public void InitiallyDisabled(Fixture fixture)
+    {
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                Setup(rec, mod);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                rec.MajorFlags |= PlacedNpc.MajorFlag.InitiallyDisabled;
+            },
+            UniquePlacedNpcAnalyzer.UniqueNpcWithoutPersistenceLocation);
+    }
+}

Antigen.Skyrim.Tests/ContextualRecords/Placed/PersistenceLocationAnalyzerTest.cs

@@ -0,0 +1,80 @@
+using Antigen.Skyrim.Record.Placed;
+using Antigen.Testing.Frameworks;
+using Mutagen.Bethesda.Skyrim;
+using Mutagen.Bethesda.Testing.AutoData;
+using Xunit;
+
+namespace Antigen.Skyrim.Tests.ContextualRecords.Placed;
+
+using Fixture = ContextualRecordTestFixture<PersistenceLocationAnalyzer, PlacedNpc, IPlacedGetter>;
+
+public class PersistenceLocationAnalyzerTest
+{
+    Cell Setup(PlacedNpc rec, ISkyrimMod mod)
+    {
+        var cell = new Cell(mod);
+        cell.Flags |= Cell.Flag.IsInteriorCell;
+        mod.Cells.AddInteriorCell(cell);
+        cell.Temporary.Add(rec);
+
+        return cell;
+    }
+
+    // An actor with a persist location must be in a cell with a location
+    [Theory, MutagenModAutoData]
+    public void CellWithoutLocation(Fixture fixture, Location location)
+    {
+        Cell? cell = null;
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                cell = Setup(rec, mod);
+                rec.PersistentLocation.SetTo(location);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                cell!.Location.SetTo(rec.PersistentLocation);
+            },
+            PersistenceLocationAnalyzer.PersistenceLocationWithCellWithoutLocation);
+    }
+
+    // A cell may inherit its location from a worldspace
+    [Theory, MutagenModAutoData]
+    public void LocationFromWorldspace(Fixture fixture, Worldspace world, Location location)
+    {
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                var cell = new Cell(mod) { Grid = new() { Point = new(0, 0) } };
+                world.AddCell(cell);
+
+                cell.Temporary.Add(rec);
+                rec.PersistentLocation.SetTo(location);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                world.Location.SetTo(rec.PersistentLocation);
+            },
+            PersistenceLocationAnalyzer.PersistenceLocationWithCellWithoutLocation);
+    }
+
+    // An npcs persist location must be the same as its cell or a parent of the cell location
+    [Theory, MutagenModAutoData]
+    public void NotInLocation(Fixture fixture, Location cellLoc, Location persistLoc)
+    {
+        Cell? cell = null;
+        fixture.Run(
+            prepForError: (rec, mod) =>
+            {
+                cell = Setup(rec, mod);
+
+                cell.Location.SetTo(cellLoc);
+                rec.PersistentLocation.SetTo(persistLoc);
+            },
+            prepForFix: (rec, mod) =>
+            {
+                cellLoc.ParentLocation.SetTo(persistLoc);
+            },
+            PersistenceLocationAnalyzer.NotInsidePersistenceLocation);
+    }
+}

Antigen.Skyrim.Tests/TopicExposureTest.cs

@@ -0,0 +1,60 @@
+using System.Reflection;
+using Autofac;
+using Antigen.SDK.Analyzers;
+using Antigen.SDK.Topics;
+using Antigen.Skyrim.Record.Conditions;
+using Antigen.Testing;
+using Mutagen.Bethesda.Plugins.Meta;
+using Noggog;
+using Shouldly;
+using Xunit;
+
+namespace Antigen.Skyrim.Tests;
+
+public class TopicExposureTest
+{
+    class TopicExposureTestData<T> : TheoryData<T>
+    {
+        public TopicExposureTestData()
+        {
+            foreach (var analyser in GetAllAnalyzers())
+            {
+                Add(analyser);
+            }
+        }
+
+        static IEnumerable<T> GetAllAnalyzers()
+        {
+            var builder = new ContainerBuilder();
+            builder.RegisterInstance(GameConstants.Get(GameRelease.SkyrimSE)).As<GameConstants>();
+            builder.RegisterModule<SkyrimAnalyzerModule>();
+            builder.RegisterModule<TestModule>();
+            var container = builder.Build();
+            return container.Resolve<IEnumerable<T>>();
+        }
+    }
+
+    IEnumerable<TopicDefinition> GetDeclaredTopics(Type type)
+    {
+        return type.GetFields(BindingFlags.Public | BindingFlags.Static | BindingFlags.GetField)
+            .Select(f => f.GetValue(null) as TopicDefinition)
+            .WhereNotNull();
+    }
+
+
+    [Theory, ClassData(typeof(TopicExposureTestData<IAnalyzer>))]
+    public void AllTopicsExposed(IAnalyzer analyzer)
+    {
+        // Special case: ConditionAnalyzer gets its topics from IConditionAnalyzer types
+        if (analyzer is ConditionAnalyzer)
+            return;
+
+        GetDeclaredTopics(analyzer.GetType()).ShouldBe(analyzer.Topics, ignoreOrder: true, customMessage: analyzer.GetType().FullName);
+    }
+
+    [Theory, ClassData(typeof(TopicExposureTestData<IConditionAnalyzer>))]
+    public void AllConditionTopicsExposed(IConditionAnalyzer analyzer)
+    {
+        GetDeclaredTopics(analyzer.GetType()).ShouldBe(analyzer.Topics, ignoreOrder: true, customMessage: analyzer.GetType().FullName);
+    }
+}

Antigen.Skyrim/Contextual/DuplicateConstructibleAnalyzer.cs

@@ -8,7 +8,7 @@ namespace Antigen.Skyrim.Contextual;
 
 public class DuplicateConstructibleAnalyzer : IContextualAnalyzer
 {
-    private static readonly TopicDefinition<int, IFormLinkNullableGetter<IConstructibleGetter>> DuplicateConstructibleReference = MutagenTopicBuilder.FromDiscussion(
+    public static readonly TopicDefinition<int, IFormLinkNullableGetter<IConstructibleGetter>> DuplicateConstructibleReference = MutagenTopicBuilder.FromDiscussion(
             206,
             "Duplicate Constructible Object",
             Severity.Warning)

Antigen.Skyrim/Extensions/CellExtensions.cs

@@ -37,6 +37,20 @@ public static IEnumerable<ILocationGetter> GetAllLocations(this ICellGetter cell
         return cellLocations;
     }
 
+    public static ILocationGetter? GetLocation(this ICellGetter cell, ILinkCache linkCache)
+    {
+        if (!cell.Location.IsNull)
+            return cell.Location.TryResolve(linkCache);
+
+        if (cell.Flags.HasFlag(Cell.Flag.IsInteriorCell))
+            return null;
+        if (!linkCache.TryResolveSimpleContext(cell, out var context))
+            return null;
+        if (!context.TryGetParent<IWorldspaceGetter>(out var world))
+            return null;
+        return world.Location.TryResolve(linkCache);
+    }
+
     /// <summary>
     /// Estimates if a cell is just a testing cell that can be ignored.
     /// A testing cell is always an interior cell, and has no special setup.

Antigen.Skyrim/Extensions/PlacedObjectExtensions.cs

@@ -136,12 +136,18 @@ public static (IScriptEntryGetter? Script, TProperty? Property) GetScriptPropert
         return (script, property?.Flags == ScriptProperty.Flag.Removed ? null : property);
     }
 
+    public static bool IsPersistent(this IPlacedGetter placed)
+    {
+        return placed.SkyrimMajorRecordFlags.HasFlag((SkyrimMajorRecord.SkyrimMajorRecordFlag)PlacedObject.DefaultMajorFlag.Persistent);
+    }
+
     public static ICellGetter? GetCell(this IPlacedGetter placed, ILinkCache linkCache, IExteriorCellCache exteriorCellCache)
     {
         if (!placed.ToLink().TryResolveSimpleContext(linkCache, out var context)) return null;
         if (context.Parent?.Record is not ICellGetter cell) return null;
 
-        if (cell.IsInteriorCell()) return cell;
+        // Persistent refs in exteriors are stored in a top level cell
+        if (cell.IsInteriorCell() || !placed.IsPersistent()) return cell;
 
         if (!context.TryGetParent<IWorldspaceGetter>(out var worldspace)) return null;
 

Antigen.Skyrim/Record/Activator/MineOreAnalyzer.cs

@@ -24,7 +24,7 @@ public class MineOreAnalyzer : IContextualRecordAnalyzer<IActivatorGetter>
             Severity.Warning)
         .WithFormatting<IMiscItemGetter>("Mine ore uses incorrect ore: {0}");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [NoMineOreScript, IncorrectVeinOre];
+    public IEnumerable<TopicDefinition> Topics { get; } = [NoMineOreScript, NoOreProperty, IncorrectVeinOre];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<IActivatorGetter> param)
     {

Antigen.Skyrim/Record/Armor/FootstepAnalyzer.cs

@@ -32,7 +32,7 @@ public class FootstepAnalyzer : IContextualRecordAnalyzer<IArmorGetter>
             Severity.Suggestion)
         .WithFormatting<IFormLinkGetter<IRaceGetter>>("Armor has multiple armor addons that have footstep sounds which are enabled for the same race {0}");
 
-    public IEnumerable<TopicDefinition> Topics => [ArmorMatchingFootstepArmorType, ArmorMissingFootstep, ArmorDuplicateFootstep];
+    public IEnumerable<TopicDefinition> Topics => [UnknownArmorType, ArmorMatchingFootstepArmorType, ArmorMissingFootstep, ArmorDuplicateFootstep];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<IArmorGetter> param)
     {

Antigen.Skyrim/Record/Armor/VendorKeywordAnalyzer.cs

@@ -19,7 +19,7 @@ public class VendorKeywordAnalyzer : IIsolatedRecordAnalyzer<IArmorGetter>
             Severity.Suggestion)
         .WithoutFormatting("Armor has unsuitable vendor keywords");
 
-    public IEnumerable<TopicDefinition> Topics => [ArmorMissingVendorKeyword];
+    public IEnumerable<TopicDefinition> Topics => [ArmorMissingVendorKeyword, UnsuitableVendorKeyword];
 
     private static readonly HashSet<FormLink<IKeywordGetter>> AllArmorVendorKeywords =
     [

Antigen.Skyrim/Record/Dialog/Branch/MissingFieldsAnalyzer.cs

@@ -18,7 +18,7 @@ public class MissingFieldsAnalyzer : IIsolatedRecordAnalyzer<IDialogBranchGetter
             Severity.Warning)
         .WithoutFormatting("Branch has no starting topic, nothing will not be available in game");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [NoQuest];
+    public IEnumerable<TopicDefinition> Topics { get; } = [NoQuest, NoStartingTopic];
 
     public void AnalyzeRecord(IsolatedRecordAnalyzerParams<IDialogBranchGetter> param)
     {

Antigen.Skyrim/Record/Npc/AmbushAnalyzer.cs

@@ -41,7 +41,7 @@ public class AmbushAnalyzer : IContextualRecordAnalyzer<INpcGetter>
             Severity.Warning)
         .WithFormatting<IPlacedNpcGetter>("Placed npc {0} with ambush script doesn't have a parent activator to trigger the ambush");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [AmbushMissingScript, AmbushNotInEditorId, AmbushAggressive, AmbushParentActivator];
+    public IEnumerable<TopicDefinition> Topics { get; } = [AmbushMissingScript, AmbushNotInEditorId, AmbushAggressive, AmbushPackages, AmbushParentActivator];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<INpcGetter> param)
     {

Antigen.Skyrim/Record/Npc/DefaultSandboxHomeownerAnalyzer.cs

@@ -12,7 +12,7 @@ public class DefaultSandboxHomeownerAnalyzer : IContextualRecordAnalyzer<INpcGet
             Severity.Suggestion)
         .WithoutFormatting("Npc uses DefaultSandboxHomeowner as last package, consider using DefaultHomeOwnerPackageList in Default Package List instead");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [];
+    public IEnumerable<TopicDefinition> Topics { get; } = [DefaultSandboxHomeownerListLast];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<INpcGetter> param)
     {

Antigen.Skyrim/Record/Npc/Level0Analyzer.cs

@@ -27,7 +27,7 @@ public class Level0Analyzer : IIsolatedRecordAnalyzer<INpcGetter>
         .WithFormatting<float>("Npc has too small level mult {0}");
 
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [Level0];
+    public IEnumerable<TopicDefinition> Topics { get; } = [Level0, MinLevel0, LevelMultTooSmall];
 
     public void AnalyzeRecord(IsolatedRecordAnalyzerParams<INpcGetter> param)
     {

Antigen.Skyrim/Record/Npc/Unique/NoCleanupScriptAnalyzer.cs

@@ -40,7 +40,13 @@ public class NoCleanupScriptAnalyzer : IContextualRecordAnalyzer<INpcGetter>
             Severity.Warning)
         .WithoutFormatting("WI quest property is not filled in cleanup script");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [NoCleanupScript, DeathContainerPropertyNotFilled, WIPropertyNotFilled];
+    public IEnumerable<TopicDefinition> Topics { get; } = [
+        NoCleanupScript,
+        DeathContainerPropertyNotFilled,
+        DeathContainerPropertyNotFilledWithContainer,
+        DeathContainerPropertyCanRespawn,
+        WIPropertyNotFilled
+    ];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<INpcGetter> param)
     {

Antigen.Skyrim/Record/Package/PackageDataAnalyzer.cs

@@ -31,7 +31,12 @@ public class PackageDataAnalyzer : IContextualRecordAnalyzer<IPackageGetter>
             Severity.Error)
         .WithFormatting<string, string?>("Package data '{0}' referenced by {1} does not exist");
 
-    public IEnumerable<TopicDefinition> Topics { get; } = [PackageWithoutOwningQuestReferencingQuestAlias, PackageReferencingMissingQuestAlias];
+    public IEnumerable<TopicDefinition> Topics { get; } = [
+        PackageWithoutOwningQuestReferencingQuestAlias,
+        PackageReferencingMissingQuestAlias,
+        PackageTargetsNoObject,
+        PackageDataDoesNotExist,
+    ];
 
     public void AnalyzeRecord(ContextualRecordAnalyzerParams<IPackageGetter> param)
     {