Multiple dataLooksValid() Calls

[ModifAmorphic]

The problem:

The dataLooksValid() method of a registered ModDataChecker class is called repeatedly for a mod archive for every directory until it comes across a directory with more than one entry (either multiple sub-directories, files or a combination of the two). At this point, it logs a Archive is not a simple archive message and makes 2 additional dataLooksValid() calls for the root "" and first directory of the archive ""root_dir. If every directory in the mod archive only contains one child directory or file , then the dataLooksValid() function is called once per until the end of the tree.

I'm not sure if this is the intended behavior. If it is, I'm struggling to understand how to effectively determine if an archive is ModDataChecker.FIXABLE without traversing the IFileTree myself on every call to the dataLooksValid() function of my custom ModDataChecker, which is pretty ineffecient. Especially if a mod archive has a long path of single children directories resulting in multiple dataLooksValid() calls for each sub-directory.

To Reproduce:

Steps to reproduce the behavior:

1. Create a new BasicGame plugin.
2. Create a simple custom ModDataChecker(GameFeature) from mobase-stubs with dataLooksValid and fix functions.
3. In the custom ModDataChecker.dataLooksValid() function, capture the path and name of the filetree: IFileTree parameter.
   qDebug(f"dataLooksValid: path=\"{filetree.path()}\", name=\"{filetree.name()}\"")
4. Register the custom ModDataChecker feature.
5. Create or use any mod archive containing a directory with mutliple subdirectories. Example:

+ root_dir
  + child_dir
    + child_child_dir
      + child_child_child_dir1
      + child_child_child_dir2

5. Expected output

dataLooksValid: path="", name=""
dataLooksValid: path="root_dir", name="root_dir"
dataLooksValid: path="root_dir/child_dir", name()="child_dir"
dataLooksValid: path="root_dir/child_dir/child_child_dir", name="child_child_dir"
dataLooksValid: path="root_dir/child_dir/child_child_dir/child_child_child_dir1", name="child_child_child_dir1"
dataLooksValid: path="root_dir/child_dir/child_child_dir/child_child_child_dir2", name="child_child_child_dir2"

6. Actual Output

[2024-12-23 00:24:23.164 D] Opened C:/ModTest/root_dir.7z using 7z (from signature).
[2024-12-23 00:24:23.169 D] dataLooksValid: path="", name=""
[2024-12-23 00:24:23.169 D] dataLooksValid: path="root_dir", name="root_dir"
[2024-12-23 00:24:23.169 D] dataLooksValid: path="root_dir\child_dir", name="child_dir"
[2024-12-23 00:24:23.169 D] dataLooksValid: path="root_dir\child_dir\child_child_dir", name="child_child_dir"
[2024-12-23 00:24:23.169 D] Archive is not a simple archive.
[2024-12-23 00:24:23.169 D] dataLooksValid: path="", name=""
[2024-12-23 00:24:23.169 D] dataLooksValid: path="root_dir", name="root_dir"
[2024-12-23 00:24:23.170 D] offering installation dialog
[2024-12-23 00:24:23.170 D] dataLooksValid: path="", name=""

Environment:

• Mod Organizer Version that exhibits the issue: v2.5.2
• Last Mod Organizer Version that did not exhibit the issue (if applicable): N/A
• Desktop OS/version used to run Mod Organizer: Windows 11

Details:

Finding difficulty trying to adapt to this behavior when writing a new plugin for a game, as you can't rely on the dataLooksValid() method to be called for every folder in the archive, as the calls stop whenever a directory with multiple children is found. So, I ended up checking all directories and files on all calls which is really inefficient and a huge drag on large archives. To somewhat combat this, I added an additional directory to the filetree which triggers the Archive is not a simple archive log entry and 4 more calls to dataLooksValid().

What I think would make more sense is if dataLooksValid() is either only called once and left up to the custom ModDataChecker to traverse the tree and figure out if a mod is valid, or dataLooksValid() is called for every directory regardless of it's child count. Either that or allow for this functionally to be toggled or somehow signaled to dataLooksValid() so the function can determine if this is the last time it's going to be asked if the mod was valid or not.

[Holt59]

This is the expected behavior, but not knowing what kind of archives you want to fix it's impossible to tell you how to work around it.

[ModifAmorphic]

I don't really need any help working around it. The purpose of this issue was to report the behavior that seemed error prone and unexpected to game plugin developers. You can see examples of people trying to work around this behavior like in the Cyberpunk plugin game_cyberpunk2077.py

    def dataLooksValid(
        self, filetree: mobase.IFileTree
    ) -> mobase.ModDataChecker.CheckReturn:
        # fix: single root folders get traversed by Simple Installer
        parent = filetree.parent()
        if parent is not None and self.dataLooksValid(parent) is self.FIXABLE:
            return self.FIXABLE

Consequently this was removed yesterday to use the "built-in" mod checker, but it was one of the examples I had previously noticed.
Mod Example:

• Deceptious Quest Core

The above Cyberpunk mod will result in 4 dataLooksValid() before Mod Organizer give's up. With the above code, this will add 2 recursive calls to dataLooksValid(), and each doing their own recursive calls until they finally get to the root. This plugin has since been updated to use the basic_mod_data_checker.py, but that doesn't really fully work around the issue either.

The basic_mod_data_checker.py also makes recursive calls to dataLooksValid, but limits it to only those folder's found in unfold: list[str]. Cyberpunk wasn't the only plugin with a recursive workaround like this. It's also present in the game_borderlands1.py and game_subnautica.py.

It's not the recursive calls to walk back "up" the file tree that's necessarily the problem, it's the recursive calls happening repeatedly for every directory until mod organizer gives up. This isn't an issue with only the custom ModDataChecker implementations either, it also happens with BasicModDataChecker when the unfold is populated.

You could use the BasicModDataChecker with unfold, but then you can't really fix mods that are fixable reliably.

Imagine a mod archive.zip:

+ MyModName
  + resources
     + modded_image.png
+ docs

Using the BasicModDataChecker and unfold set as:

BasicModDataChecker(
  GlobPatterns(
      unfold=["resources" ],
      valid=[ "*.png" ]
  )
)

The above archive will not be fixed simply because of the docs folder at the root, even though the archive is perfectly fixable. So, if the plugin author decides to try and fix this on their own they can, but then they have to deal with the fact that their custom dataLooksValid is going to get called multiple times potentially when installed. Oddly enough, the simple installer doesn't seem behave this way. It only appears to make a single call to dataLooksValid per edit, although maybe that's because some flag was tripped earlier when the mod archive traversal was originally attempted?

I think what is happening in reality is everyone gives up trying to fix all archives and resigns to having the mod author or installer fix it. Probably not a huge issue with for games with a lot of active mod development, but for older games that means it's on the user to fix them.

I guess what I'm trying to understand is what the use case is for calling dataLooksValid multiple times actually is? Seems simpler to me to either leave it up to the plugin author to implement, or traverse every directory. But doing a mix of both is just confusing.

[Holt59]

I think what is happening in reality is everyone gives up trying to fix all archives and resigns to having the mod author or installer fix it. Probably not a huge issue with for games with a lot of active mod development, but for older games that means it's on the user to fix them.

The main idea of mod-data checker and fix is to fix standard stuff, not to try to re-organize whole archive. People have tried to do so with basic mod data checker, etc., etc., but in the end, it's impossible to have something that can fix any archives because mod authors can make any kind of weird structure in archives.

I guess what I'm trying to understand is what the use case is for calling dataLooksValid multiple times actually is? Seems simpler to me to either leave it up to the plugin author to implement, or traverse every directory. But doing a mix of both is just confusing.

This is exactly to fix many of the badly structure mod archives created for "older" games (e.g., Morrowind / Oblivion). Many mod authors have a tendency to create archive like this:

Some Picture.png
Mod Name/
    README.txt
    Data/
        mymod.esp

...thus the only way to find the underlying ESP file is to recurse into sub-folders until one sub-folder is found to be valid, and the only way to know if a folder is valid is to call dataLooksValid.

[ModifAmorphic]

Thanks for the explanation. I realize my comment was quite long, but the point I was trying to make was I don't understand how the current behavior of Mod Organizers recursing through some directories solves anything though, including your example. I understand fully why recursing through directories is necessary for a plugin to do.

I created an archive based on your example and used my simple ModDataChecker that doesn't do anything but log.

Some Picture.png
Mod Name/
    README.txt
    Data/
        mymod.esp

This is the output from my very simple dataLooksValid that essentially just logs output.

[2024-12-23 18:58:08.533 D] using mod name "complex_root" (id 0) ->C:/temp/complex_root/complex_root.7z
[2024-12-23 18:58:08.533 D] Opened C:/temp/complex_root/complex_root.7z using 7z (from signature).
[2024-12-23 18:58:08.537 D] dataLooksValid: path="", name=""
[2024-12-23 18:58:08.537 D] Archive is not a simple archive.
[2024-12-23 18:58:08.537 D] dataLooksValid: path="", name=""
[2024-12-23 18:58:08.537 D] dataLooksValid: path="Mod Name", name="Mod Name"
[2024-12-23 18:58:08.537 D] offering installation dialog
[2024-12-23 18:58:08.538 D] dataLooksValid: path="", name=""

So Mod Organizer isn't making a calls far enough to make it to the 'Mod Name/Data' folder, so why bother with it? Just call dataLooksValid once and be done with it. This seems to be ok for the built-in installer, as far as I can tell it only ever makes one call per edit.

[Holt59]

Yes actually the installer will only recurse if there is a single folder (not picture or text), the picture or text stuff is to check for a data folder, but the idea is kind of the same.

You can check all the logic here: https://github.com/ModOrganizer2/modorganizer-installer_quick/blob/master/src/installerquick.cpp#L66-L112