-
-
Notifications
You must be signed in to change notification settings - Fork 99
Preserve exclusion/include list and add Progress bar for local mod installation #667
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
base: master
Are you sure you want to change the base?
Preserve exclusion/include list and add Progress bar for local mod installation #667
Conversation
…yPackagesFromExtractFolderToTargetDir async to stop UI freezes for larger mods.
…Wrapper was modified The config used to only be updated once an item was added or deleted.
|
Also fixed a bug where adding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than translatable string usage
Makes it so they don't come back everytime you open the publish window if you remove them (though I have no idea why anyone would remove these regexes)
| /// <summary> | ||
| /// ViewModel for downloading an individual package. | ||
| /// </summary> | ||
| public class InstallPackageViewModel : INotifyPropertyChanged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use autogeneration for INotifyPropertyChanged here.
It's been a while since I've worked on R2 but I believe this uses Fody.PropertyChanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the way the rest of the project does this IIRC. Have a quick peek around.
|
|
||
| // Set default Regexes. | ||
| IgnoreRegexes = new ObservableCollection<StringWrapper>() | ||
| IgnoreRegexes = new ObservableCollection<StringWrapper>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not overkill?
You can just save it when the menu is closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might also save you from unnecessarily implementing PropertyChanged in the other place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this still needs moving.
Sorry for the bad commit names, it's a bit late for me rn
|
I will clean this up later- god knows when, but later- still a bit of unneeded AI junk remaining. |
No description provided.