Mod.plugins is a list of Paths instead of strs.

When Mod.plugins was a list of strings, we didn't have enough
information to safely delete plugins without doing a ton of validation
that those plugin files were actually under Data. This validation code
was messy, and I wanted to keep the logic that detects whether a file is
a plugin isolated to Mod.__post_init__.

Make Mod.plugins a list of Paths. This allows us to greatly simplify the
plugin deletion logic and still avoid deleting files we aren't supposed
to.

In removing this validation, the logic that determines whether a file is
appropriately isolated to Mod.__post_init__.

ammo/component.py

@@ -104,7 +104,7 @@ def __post_init__(self) -> None:
             if plugin_dir.exists():
                 for f in plugin_dir.iterdir():
                     if f.suffix.lower() in (".esp", ".esl", ".esm") and not f.is_dir():
-                        self.plugins.append(f.name)
+                        self.plugins.append(f)
 
 
 @dataclass(kw_only=True, slots=True)

ammo/mod_controller.py

@@ -158,7 +158,7 @@ def __init__(self, downloads_dir: Path, game: Game, *keywords):
                 for m in self.mods[::-1]:
                     if not m.enabled:
                         continue
-                    if name in m.plugins:
+                    if name in (i.name for i in m.plugins):
                         mod = m
                         break
 
@@ -390,30 +390,30 @@ def _set_component_state(self, component: ComponentEnum, index: int, state: bool
             subject.enabled = state
             if subject.enabled:
                 # Show plugins owned by this mod
-                for name in subject.plugins:
-                    if name not in [i.name for i in self.plugins]:
+                for subject_plugin in subject.plugins:
+                    if subject_plugin.name not in (i.name for i in self.plugins):
                         plugin = Plugin(
-                            name=name,
+                            name=subject_plugin.name,
                             mod=subject,
                             enabled=False,
                         )
                         self.plugins.append(plugin)
             else:
                 # Hide plugins owned by this mod and not another mod
                 for plugin in subject.plugins:
-                    if plugin not in [i.name for i in self.plugins]:
+                    if plugin.name not in (i.name for i in self.plugins):
                         continue
                     provided_elsewhere = False
                     for mod in self.mods:
                         if not mod.enabled:
                             continue
                         if mod == subject:
                             continue
-                        if plugin in mod.plugins:
+                        if plugin.name in (i.name for i in mod.plugins):
                             provided_elsewhere = True
                             break
                     if not provided_elsewhere:
-                        index = [i.name for i in self.plugins].index(plugin)
+                        index = [i.name for i in self.plugins].index(plugin.name)
                         self.plugins.pop(index)
 
         # Handle plugins
@@ -607,8 +607,8 @@ def sort(self) -> None:
             if not mod.enabled:
                 continue
             for plugin in self.plugins[::-1]:
-                for plugin_name in mod.plugins:
-                    if plugin.name == plugin_name and plugin.name not in (
+                for plugin_file in mod.plugins:
+                    if plugin.name == plugin_file.name and plugin.name not in (
                         i.name for i in plugins
                     ):
                         plugins.insert(0, plugin)
@@ -769,18 +769,9 @@ def get_plugin_files(plugin):
                     for mod in self.mods:
                         if not mod.enabled:
                             continue
-                        if plugin.name in mod.plugins:
-                            for file in mod.files:
-                                if all(
-                                    (
-                                        file.name == plugin.name and not file.is_dir(),
-                                        file.parent == mod.location
-                                        or file.parent.name.lower()
-                                        == self.game.data.name.lower(),
-                                    )
-                                ):
-                                    yield file
-                                    break
+                        for file in mod.plugins:
+                            if file.name == plugin.name:
+                                yield file
 
                 if index == "all":
                     deleted_plugins = ""
@@ -797,7 +788,7 @@ def get_plugin_files(plugin):
                         ):
                             self.refresh()
                             self.commit()
-                        self.plugins.pop(self.plugins.index(plugin))
+                        self.plugins.remove(plugin)
                         for file in get_plugin_files(plugin):
                             try:
                                 file.unlink()
@@ -1108,7 +1099,7 @@ def find(self, *keyword: str) -> None:
             # We can't simply plugin.mod.visible = True because plugin.mod
             # does not care about conflict winners. This also means we can't break.
             for mod in self.mods:
-                if plugin.name in mod.plugins:
+                if plugin.name in (i.name for i in mod.plugins):
                     mod.visible = True
 
         if len(self.keywords) == 1:

test/common.py

@@ -299,13 +299,13 @@ def install_mod(controller, mod_name: str):
     controller.activate(ComponentEnum.MOD, mod_index)
 
     for plugin in controller.mods[mod_index].plugins:
-        plugin_index = [i.name for i in controller.plugins].index(plugin)
+        plugin_index = [i.name for i in controller.plugins].index(plugin.name)
         controller.activate(ComponentEnum.PLUGIN, plugin_index)
 
     controller.commit()
     assert controller.mods[mod_index].enabled is True
     for plugin in controller.mods[mod_index].plugins:
-        plugin_index = [i.name for i in controller.plugins].index(plugin)
+        plugin_index = [i.name for i in controller.plugins].index(plugin.name)
         assert controller.plugins[plugin_index].enabled is True
 
     return mod_index

test/test_controller_instance.py

@@ -449,10 +449,13 @@ def test_controller_delete_plugin():
     Test that deleting a plugin removes the plugin from
     the parent mod's files somewhere under ~/.local/share/ammo
     and doesn't leave a broken symlink in the game dir.
+
+    Tests that it doesn't delete files if they weren't loaded as plugins
+    (like .esp files that weren't under Data).
     """
     with AmmoController() as controller:
         # Install a mod with a plugin, ensure the plugin is there.
-        install_mod(controller, "normal_mod")
+        install_mod(controller, "mult_plugins_same_name")
         assert len(controller.plugins) == 1
 
         # Delete the plugin, make sure it's gone.
@@ -465,6 +468,13 @@ def test_controller_delete_plugin():
         # Ensure the plugin hasn't returned.
         assert len(controller.plugins) == 0
 
+        # Ensure we didn't delete .esp files that weren't under Data
+        assert (
+            controller.game.ammo_mods_dir
+            / "mult_plugins_same_name/Data/test/plugin.esp"
+        ).exists()
+        assert (controller.game.data / "test/plugin.esp").exists()
+
 
 def test_controller_plugin_wrong_spot():
     """

test/test_validation.py

@@ -103,7 +103,7 @@ def test_deactivate_validation():
     with AmmoController() as controller:
         mod_index = install_mod(controller, "normal_mod")
         plugin_index = [i.name for i in controller.plugins].index(
-            controller.mods[mod_index].plugins[0]
+            controller.mods[mod_index].plugins[0].name
         )
 
         # valid deactivate plugin