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

[Feature] Alternate MiniAVC handling idea #2490

Closed
HebaruSan opened this issue Aug 7, 2018 · 23 comments · Fixed by #3458
Closed

[Feature] Alternate MiniAVC handling idea #2490

HebaruSan opened this issue Aug 7, 2018 · 23 comments · Fixed by #3458
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI

Comments

@HebaruSan
Copy link
Member

HebaruSan commented Aug 7, 2018

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 Warn before launching KSP with installed incompatible modules #2601.

The filter would be easy to add here:

foreach (ZipEntry entry in zipfile)
{
// Skips things not prescribed by our install stanza.
if (!stanza.IsWanted(entry.Name))
{
continue;
}
// Prepare our file info.
InstallableFile file_info = new InstallableFile
{
source = entry,
makedir = makeDirs,
destination = null
};
// If we have a place to install it, fill that in...
if (installDir != null)
{
// Get the full name of the file.
string outputName = entry.Name;
// Update our file info with the install location
file_info.destination = TransformOutputName(stanza.file, outputName, installDir, stanza.@as);
}
files.Add(file_info);
}

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.

@HebaruSan HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN labels Aug 7, 2018
@linuxgurugamer
Copy link
Contributor

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

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
Copy link
Contributor

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

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
Copy link
Contributor

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

I just realized you maintain both MiniAVC and ZeroMiniAVC now. 😁

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

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

damccull commented Mar 8, 2019

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

Some updates on this...

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):

image

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

@HebaruSan HebaruSan changed the title [Feature suggestion] Alternate MiniAVC handling idea [Feature] Alternate MiniAVC handling idea Sep 26, 2019
@HebaruSan
Copy link
Member Author

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

@HebaruSan
Copy link
Member Author

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

HebaruSan commented May 21, 2021

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:

image

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

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

image

@linuxgurugamer
Copy link
Contributor

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

@linuxgurugamer
Copy link
Contributor

The MiniAVC-V2 zip file has been fixed on Github

@HebaruSan
Copy link
Member Author

HebaruSan commented Jun 3, 2021

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

image

@HebaruSan
Copy link
Member Author

HebaruSan commented Sep 26, 2021

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

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

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

image

image

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

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants