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

Mass Metadata Parsing #2079

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

KenJyn76
Copy link

I liked the idea of #1861, but could not get it built. Fixed up and implemented the functionality started over there. Adds a button opposite the Refresh button in the Downloads tab that queries every mod in the Downloads list that does not have a .meta file associated with it.

@KenJyn76
Copy link
Author

Screenshot 2024-07-27 220204

This is what it looks like. I recorded a demo but couldn't get it uploaded. Sometimes it will miss a file here or there, but a rerun will catch them if they can be queried.

Deewens and others added 3 commits July 29, 2024 10:10
Added button to pr

Update mainwindow.ui

Update mainwindow.ui

Added button to pr
Fixed clang formatting on downloadstab.cpp
@Holt59
Copy link
Member

Holt59 commented Jul 29, 2024

This should probably display some kind of popup if there are more than a few missing files (maybe 5 or something like that) because 1) this could lead to bad performances due to the directory watcher - queryInfoMd5 only triggers the request, it is non-blocking, so the startDisableDirWatcher is not useful here, and 2) this can consume many API requests.

@KenJyn76
Copy link
Author

This should probably display some kind of popup if there are more than a few missing files (maybe 5 or something like that) because 1) this could lead to bad performances due to the directory watcher - queryInfoMd5 only triggers the request, it is non-blocking, so the startDisableDirWatcher is not useful here, and 2) this can consume many API requests.

Added a warning to show the user how many files will be fetched, and that it will cause stutter and consume API requests. Not disabling the dir watcher while querying metadata can cause a silent crash, which doesn't seem to be present when disabling it

@Holt59
Copy link
Member

Holt59 commented Jul 29, 2024

This should probably display some kind of popup if there are more than a few missing files (maybe 5 or something like that) because 1) this could lead to bad performances due to the directory watcher - queryInfoMd5 only triggers the request, it is non-blocking, so the startDisableDirWatcher is not useful here, and 2) this can consume many API requests.

Added a warning to show the user how many files will be fetched, and that it will cause stutter and consume API requests. Not disabling the dir watcher while querying metadata can cause a silent crash, which doesn't seem to be present when disabling it

Your change makes it impossible to refresh for less than 5 files I think.

The new dialog box hijacked the function and skipped the fetch if there were less than 5 files. Now directly runs if there are 5 files or less, and shows the dialog if there are more than 5\
@KenJyn76
Copy link
Author

This should probably display some kind of popup if there are more than a few missing files (maybe 5 or something like that) because 1) this could lead to bad performances due to the directory watcher - queryInfoMd5 only triggers the request, it is non-blocking, so the startDisableDirWatcher is not useful here, and 2) this can consume many API requests.

Added a warning to show the user how many files will be fetched, and that it will cause stutter and consume API requests. Not disabling the dir watcher while querying metadata can cause a silent crash, which doesn't seem to be present when disabling it

Your change makes it impossible to refresh for less than 5 files I think.

My bad. Should be fixed now

@Silarn
Copy link
Member

Silarn commented Jul 29, 2024

Obviously not every file in the download pane necessarily has to come from Nexus. I think it may be worth adding a metadata flag to indicate if it was already queried. This should avoid running checks on non-Nexus files after an attempt has been made.

@Silarn
Copy link
Member

Silarn commented Jul 29, 2024

What we probably should do is allow for some multiselect context options so grouping of files can be checked and not necessarily the entire list. Although that's not necessarily in the scope of this change.

@KenJyn76
Copy link
Author

KenJyn76 commented Aug 1, 2024

The original PR I built off of mentioned that as well, with this being a stopgap measure. Full transparency, this is my first time reading or writing C++ or using Qt so I'm totally lost in the metadata writing functions. As far as I can see, the best way to do this would be to make a stub fileName.meta with the contents

[General]
repository=NOT_FOUND

If checks are failed on the Mod ID. It feels like this should be added into the QueryInfo functions in general, though, rather than tied to the button.

@kcrrr
Copy link

kcrrr commented Aug 20, 2024

This is a dumb question, but is there any way for me, as someone who knows nothing about coding, to put this into my MO2 setup? Could really use this feature as I'm moving my Skyrim setup from NMM over to MO2.

@Silarn
Copy link
Member

Silarn commented Aug 26, 2024

@kcrrr You'd have to build the mo2 application with this edit. You could always try to set that up via https://github.com/ModOrganizer2/mob

@KenJyn76 To be clear I'd like to see the addition of the metadata flag before I would personally approve this in order to mitigate unnecessary repeat queries. The multiselect bit can be added later.

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.

5 participants