[Feature] Alternate MiniAVC handling idea

HebaruSan commented 6 years ago

Background

MiniAVC is a mod for modders that automatically checks whether a mod is compatible with the current game version and displays a warning if it isn't. It can also check a remote URL for new versions of a mod. It's related to KSP-AVC, and it parses .version files.

Many mods bundle a copy of MiniAVC. Mod authors see this as a way to reduce problems caused by incompatible game versions. Currently CKAN's policy is to install those bundled MiniAVC DLLs.

Problem

Some users may consider MiniAVC redundant with CKAN and not helpful. CKAN can already tell you whether there are updates and whether your mods are compatible, as well as installing the updates for you. If you're aware of the risk and want to try your incompatible mods anyway, the extra warnings when launching the game are kind of annoying to click through.

Evidence of this is the existence of ZeroMiniAVC.

Previous developments

KSP-CKAN/NetKAN#1455 outlines the history of CKAN's attempts to deal with MiniAVC. There was an effort to support it as a standalone mod, which can't work because it uses the DLL's location to determine the scope of scanning:

If you only install it to GameData/MiniAVC/MiniAVC.dll, it will only scan the MiniAVC folder for .version files, so you won't be warned about incompatible mods or available updates
If you install it to GameData/MiniAVC.dll, it will scan the entire GameData folder, including mods that don't want MiniAVC support, and including manually installed mods with bundled copies of MiniAVC that then cause messages to be displayed multiple times

To work properly, MiniAVC.dll has to be exactly where a bundling mod puts it. This means we can't share it between modules.

Suggestions

In the settings window, add a checkbox titled, "Suppress MiniAVC"
When installing a module, check the setting. If it's checked, then don't install any files named MiniAVC.dll, MiniAVC.xml, or LICENSE-MiniAVC.txt
~~To serve MiniAVC's purpose, we should add a check to the Launch KSP button: scan the installed modules and warn the user if any aren't compatible with the current game version.~~ Done in #2601.

The filter would be easy to add here:

https://github.com/KSP-CKAN/CKAN/blob/ab714c224b9b446ac4a6d9c9b209b73f14bfa752/Core/ModuleInstaller.cs#L514-L541

Since concerns have been raised about whether de-bundling MiniAVC violates modders intentions, the checkbox should be unchecked by default. CKAN would continue to install bundled MiniAVC by default for new and upgrading users, and only if a user was aware of the new setting and willing to take on the risk of enabling it would mods be installed differently.

linuxgurugamer commented 6 years ago

The problem with this, is that CKAN only notifies you when you do a refresh. MiniAVC checks every time the game is started.

Also, many people just start the game directly, or through a shortcut. Again, CKAN is useless in this case.

I would possibly support the idea that if the full KSP-AVC is installed, to then disable the installation of MiniAVC, otherwise I think this a very bad idea

HebaruSan commented 6 years ago

I think those concerns are addressed by making it optional, and turned off by default. The normal scenario for a user who just downloads and runs ckan.exe would be: auto-refresh on launch and all bundled instances of MiniAVC installed, just as it is today. This would just be a quality of life option for those who want it.

linuxgurugamer commented 6 years ago

I would still make that dependent on the full KSP-AVC being installed, mainly because you are circumventing mod authors intentions. As a mod author, I put MiniAVC in along with the .version to be sure people are notified of updates. And, for those people who don't want it, there IS ZeroAVC

HebaruSan commented 6 years ago

But as a user, I don't want either of them in my GameData. ZeroMiniAVC deletes files that CKAN installed, and I'd rather they not be installed in the first place.

linuxgurugamer commented 6 years ago

Not going to be fanatical about this, but consider that when people install mods by hand, they first copy the whole directory, and then, possibly, delete the MiniAVC.dll. My opinion, is that CKAN should do what the mod authors specify in the .netkan files. Anything beyond is out-of-scope

HebaruSan commented 6 years ago

I just realized you maintain both MiniAVC and ZeroMiniAVC now. :grin:

Any thoughts on possibly updating MiniAVC to work better in a standalone installation by CKAN? Maybe the Gamedata-level DLL could check for other MiniAVC.dll files before running its checks? That should avoid raising multiple warnings for the same mod, making the GameData-level DLL a better option.

HebaruSan commented 6 years ago

Idea for making this more generic: Instead of a checkbox, have a global "Installation filter" regex setting in a text box. If set to MiniAVC\.dll|MiniAVC\.xml|LICENSE-MiniAVC\.txt, it would have the same effect. This would avoid making a setting just for one specific mod.

damccull commented 5 years ago

I like the idea of removing redundant files. The best case would be for the AVC's to be updated and just install directly under gamedata, then look for some kind of flag on a mod before checking it for updates.

However, from a CKAN perspective, I would prefer to not see the AVC notifications on game start, and NOT have to manually remove them. If they could be suppressed automatically (no matter how I launch the game) I'd enjoy that. I don't update CKAN every time I launch the game, and normally launch it from steam, but I check ckan every few days for updates.

HebaruSan commented 5 years ago

Some updates on this...

linuxgurugamer/KSPAddonVersionChecker#13 updated MiniAVC to work better as a standalone install in the GameData root, as of AVC 1.2.0.3
2601 added a warning to CKAN if you launch KSP via CKAN with known incompatible mods installed, covering part of what MiniAVC is for, landing in CKAN 1.26

With these changes, it ought to be safe to:

Index MiniAVC as a standalone mod again
Filter MiniAVC out of mods where it's bundled
Add MiniAVC to the recommends list for those mods

This would let users choose whether they want to use MiniAVC without having to create special settings for it.

However, MiniAVC's releases are currently in an un-indexable form (lots of assets with probably unworkable module versions):

So we'd have to figure out how to deal with that.

HebaruSan commented 4 years ago

Raised linuxgurugamer/KSPAddonVersionChecker#33 because that just doesn't make sense anyway.

HebaruSan commented 4 years ago

As a mod author, I put MiniAVC in along with the .version to be sure people are notified of updates.

LOL KSP-CKAN/NetKAN#7690

HebaruSan commented 3 years ago

And, for those people who don't want it, there IS ZeroAVC

My understanding is that this is intentionally no longer the case. ZeroMiniAVC does not remove the latest versions of MiniAVC, because it is now being used as a clean-up tool for outdated versions instead of its original purpose of implementing a user preference of not using MiniAVC:

So the idea in the OP here is now more useful, since it could filter out all versions of MiniAVC if the user so chose.

HebaruSan commented 3 years ago

Further update, MiniAVC is now down to one asset per GitHub release, so indexing it in CKAN should be eminently feasible:

https://github.com/linuxgurugamer/KSPAddonVersionChecker/releases

linuxgurugamer commented 3 years ago

It wasn't intended that MiniAVC be installed by itself. however, if this will help solve some problems, I can fix that

linuxgurugamer commented 3 years ago

The MiniAVC-V2 zip file has been fixed on Github

HebaruSan commented 3 years ago

if this will help solve some problems, I can fix that

Thanks, but to be clear, it won't help solve other problems, it was just a weird thing I noticed while looking at that ZIP. I try to submit reports when I notice things that aren't quite right.

HebaruSan commented 3 years ago

a global "Installation filter" regex setting in a text box. If set to MiniAVC\.dll|MiniAVC\.xml|LICENSE-MiniAVC\.txt, it would have the same effect.

Now I'm thinking a list of paths would be better, since then a user could enter this without knowledge of regex syntax:

MiniAVC.dll
MiniAVC.xml
LICENSE-MiniAVC.txt
MiniAVC-V2.dll
MiniAVC-V2.dll.mdb

For ease of use, we could add a "Add MiniAVC" button near the list to default those in. And of course it should be possible to enter full paths, to help with #3129's request to filter certain files from certain mods.

etmoonshade commented 3 years ago

Just because I got the ping over on #3129, figured I should comment here as well.

Obviously I'm a big fan of the idea of an installation filter - I kinda opened that issue. :)

With that said, it feels to me like this should be decoupled (in the UI, that is) from the installation filter. It's a power user mode IMO, and activating the filter in the UI should come with plenty of disclaimers like "THIS CAN BREAK YOUR GAME AND HURT THE ENTIRE TIME YOU ARE BREAKING IT."

By contrast, blocking MiniAVC is a very focused set of blocks that are likely to be widely used. Unless some mod author names their rover-sized Autonomous Vehicle Controller mod "MiniAVC.dll," I can't see how it'll break anything.

HebaruSan commented 3 years ago

Borrowing the layout of the existing Settings → CKAN Plugins window:

Hopefully the typical novice user would be intimidated and run away without copious warnings, unless they've been told to click the MiniAVC button. We've got a lot of room for more buttons on the right side, I'm thinking about adding Clear, but I'm not sure what else might be useful.

etmoonshade commented 3 years ago

On a functionality note, wildcards could be useful here. That way you could exclude "MiniAVC." or "WarpPlugin/Patches/*" for example (not that the latter would be useful, but I can think of places where I'd want to wildcard an entire directory as in my comment on #3129)

etmoonshade commented 3 years ago

Side question - should #3129 be merged into this one at this point, since it looks like you're implementing that request to solve this one? Or is there a reason not to?

HebaruSan commented 3 years ago

On a functionality note, wildcards could be useful here. That way you could exclude "MiniAVC*." or "WarpPlugin/Patches/" for example (not that the latter would be useful, but I can think of places where I'd want to wildcard an entire directory as in my comment on #3129)

On the wildcard front, that's a good use case, but I'm trying to stay away from making the user learn special syntax. My plan was to check whether the value from the filter box matches any substring of the full path of the install file, so WarpPlugin/Patches would indeed match everything in that folder, without having to define a special character (and MiniAVC.dll would match that filename in any folder).

Side question - should #3129 be merged into this one at this point, since it looks like you're implementing that request to solve this one? Or is there a reason not to?

I see them as two distinct requests, since technically it would be possible to address one without the other, but that could be satisfied by the same set of changes if done right.

KSP-CKAN / CKAN