Skip to content

Conversation

TheBestAstroNOT
Copy link
Contributor

No description provided.

@TheBestAstroNOT TheBestAstroNOT changed the title Preserve exclusion/include list Preserve exclusion/include list and add Progress bar for local mod installation Aug 9, 2025
…Wrapper was modified

The config used to only be updated once an item was added or deleted.
@TheBestAstroNOT
Copy link
Contributor Author

TheBestAstroNOT commented Aug 11, 2025

Also fixed a bug where adding $"Regex.Escape($@"{_modTuple.Config.ModId}.nuspec")}" and Regex.Escape(ModConfig.ConfigFileName) didn't save the config.

Copy link
Contributor

@dreamsyntax dreamsyntax left a 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
Copy link
Member

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.

Copy link
Member

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>(
Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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
@Sewer56
Copy link
Member

Sewer56 commented Oct 8, 2025

I will clean this up later- god knows when, but later- still a bit of unneeded AI junk remaining.

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.

3 participants