Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip .git dirs & .mohidden files #60

Closed
wants to merge 7 commits into from
Closed

Skip .git dirs & .mohidden files #60

wants to merge 7 commits into from

Conversation

Twinki14
Copy link

Authors

Modifications

Skips .git directories and .mohidden files to speed up USVFS loading, this can also make UI refreshes quicker

Copy link
Member

@Silarn Silarn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main problem with this approach is that we're still attempting to maintain USVFS as a generally separate utility from MO2. So these direct MO2-specific features are a no-go.

What would need to be done here would be to implement a blacklist feature where you could pass directories or file patterns when MO2 runs USVFS and then have USVFS ignore the blacklisted directories / files.

@Twinki14 Twinki14 closed this May 14, 2024
@AnyOldName3
Copy link
Member

There are also other things we've got on the MO2 tracker that we can't implement without a directory and file blacklist, like profile-specific saved games for games that don't have a convenient INI setting to change the save directory location, so that approach would have extra utility, too.

@Holt59
Copy link
Member

Holt59 commented May 15, 2024

I don't think a blacklist would be sufficient for profile-specific save games, there are two use-cases

  • Ignore specific directories/files when mapping directories (not meaningful when mapping files). This could actually be done in MO2 if we mapped files instead of directories but that would be pretty inefficient. This is probably not a huge change on USVFS.
  • Hide files from the target directory, i.e., hiding main saves when using profile saves. That's much more complicated on the USVFS side since this would require changes to many wrapped-functions to handle overriding hidden files.

@AnyOldName3
Copy link
Member

Hide files from the target directory, i.e., hiding main saves when using profile saves. That's much more complicated on the USVFS side since this would require changes to many wrapped-functions to handle overriding hidden files.

That's what I was interpreting blacklist to mean - I was expecting that MO2 would pass in the paths of all .git directories and .mohidden files rather than passing in a list of banned file names and extensions that USVFS would check everything against.

@Holt59
Copy link
Member

Holt59 commented May 15, 2024

Hide files from the target directory, i.e., hiding main saves when using profile saves. That's much more complicated on the USVFS side since this would require changes to many wrapped-functions to handle overriding hidden files.

That's what I was interpreting blacklist to mean - I was expecting that MO2 would pass in the paths of all .git directories and .mohidden files rather than passing in a list of banned file names and extensions that USVFS would check everything against.

Regardless of what MO2 gives to USVFS, that's two very different behaviors, so I don't think you can handle both of them properly from USVFS.

Not sure if we're already saying the same thing but I will clarify my point.

Consider the two directories:

X/
  .git/
  foo
  bar

Y/
  foo
  baz

If you currently map X to Y in USVFS, proxied applications will see .git, foo, baz and bar in Y (with foo being from X I think).

  • The first "blacklist" use-case is to not map .git, so that proxied applications could only see foo, bar and baz. That's (IMO) a pretty easy and small change to USVFS. That could also be done directly in MO2 by mapping X/foo and X/bar directly to Y/foo and Y/bar, but that would be very inefficient.
  • The second "blacklist" use-case is to hide Y/foo and Y/baz completely, so that proxied applications would only see .git, foo and bar, and if the proxied application request write to Y/foo or Y/baz, it would instead write to X/foo and X/baz. It's actually not "blacklist", it's more like "full redirect", i.e., completely override (for reading and writing) the target directory.

@AnyOldName3
Copy link
Member

For context, I know very little about the specific internals of USVFS, but from knowing vague details about its operation and other stuff about VFSes in general and how MO2 exposes stuff, I'd have expected the implementation for the two behaviours to be very similar. I'm under the impression the real Y isn't treated any differently from the real X as USVFS already has to intercept everything and return its own alternative. For a which files are in Y call, USVFS already has to know about the files in the real Y to include them in the list with the extras from other sources. If that's the case, supporting include the real Y without Y/foo isn't any different to supporting include the real X without X/foo, and whether or not my assumption's true, include the real X without X/foo is very similar to include the real X without anything called foo as it's just how much of the path we're comparing that's different. If I were implementing my own VFS (like that time I already did) I'd implement it in line with the assumption I'm making here as it avoids having two different code paths depending on whether a file's mapped to its original path or not.

@Silarn
Copy link
Member

Silarn commented May 15, 2024

I believe, for the purposes of this change, we're merely discussing ignoring mapped directories or files coming from MO2. This could be done in MO2 but then we'd be complicating the mapping process since we're generally only concerned about the top level directories on the MO2 side. USVFS then crawls through the directories as it translates directories into mapped file locations. I think being able to remove specific file patterns or directory names from the mapping process in USVFS does have its uses, not just to avoid spending extra time on files we don't need to worry about mapping (as per the original PR here).

Being able to hide files from the original directories is definitely a more complicated process as we'd need to modify most of the hooks to essentially avoid returning data about the files in those locations and only produce file handles for mapped locations. This is doable but not necessarily the same as a basic mapping blacklist.

@Holt59
Copy link
Member

Holt59 commented May 15, 2024

For context, I know very little about the specific internals of USVFS, but from knowing vague details about its operation and other stuff about VFSes in general and how MO2 exposes stuff, I'd have expected the implementation for the two behaviours to be very similar. I'm under the impression the real Y isn't treated any differently from the real X as USVFS already has to intercept everything and return its own alternative. For a which files are in Y call, USVFS already has to know about the files in the real Y to include them in the list with the extras from other sources. If that's the case, supporting include the real Y without Y/foo isn't any different to supporting include the real X without X/foo, and whether or not my assumption's true, include the real X without X/foo is very similar to include the real X without anything called foo as it's just how much of the path we're comparing that's different. If I were implementing my own VFS (like that time I already did) I'd implement it in line with the assumption I'm making here as it avoids having two different code paths depending on whether a file's mapped to its original path or not.

I'm not much familiar with USVFS, but from my understand, the first cases can simply be done by not adding the files/directories to ignore to the virtual file-tree maintained by USVFS.

In the second case, you actually need to modify the way hooks work since, when a file is not mapped, USVFS falls back to the original file, so you would need to check if the file is part of a "fully redirected" directory, and if so, redirect to the mapped directory.

@Silarn
Copy link
Member

Silarn commented May 15, 2024

The example use-case here, not just about a theoretical .git directory (which would be a silly thing to have in a mod, but I guess it could happen), removing .mohidden files from the file mapping actually makes a lot of sense. Because the way it currently works, these files are technically there and visible to the application being hooked. They're just not used because they've been renamed to something the application doesn't care about.

If you have a bunch of textures with overlaps and you have to hide a handful from several mods to get the exact combination you care about, now USVFS has to consider a bunch of extra overlapping file conflicts for files that ultimately don't matter. It would be better if it just ignored them completely.

So that's why I'm generally 'for' this change, if it can be implemented properly.

@Twinki14
Copy link
Author

I spent some time experimenting last night with a more proper implementation, and as @qudix had suggested in the discord, a similar implementation to

DLLEXPORT VOID WINAPI BlacklistExecutable(LPWSTR executableName);
would be a very reasonable implementation, which I plan to follow through with for a future new PR

@Silarn
Copy link
Member

Silarn commented May 15, 2024

Sounds good. I think that's the best approach. I personally think having two such methods would be ideal. One for directory names and one for file pattern matching (be that a glob or regex). I see less overall need for partial matching for directories while being able to match file extensions would be ideal (but also having the ability to block specific file names if needed).

@Twinki14
Copy link
Author

Twinki14 commented May 15, 2024

Sounds good. I think that's the best approach. I personally think having two such methods would be ideal. One for directory names and one for file pattern matching (be that a glob or regex). I see less overall need for partial matching for directories while being able to match file extensions would be ideal (but also having the ability to block specific file names if needed).

I wanted to avoid glob or regex just from a purely performance concern, but the performance hit with regex might just be negligible. If you have any thoughts on that please air them, I'm not that experienced in MO2 to know if it'd be a big hit (unless it's implemented poorly). I was going to go for a very simple custom wildcard pattern, eg *.git, readme.txt, *.txt, *.mohidden, someEsp.mohidden.

Also, would customizing the list be better to have on a "global" MO2 setting, or a plugin level setting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants