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

MiniAVC Policy #1455

Closed
TeddyDD opened this issue May 30, 2015 · 20 comments
Closed

MiniAVC Policy #1455

TeddyDD opened this issue May 30, 2015 · 20 comments

Comments

@TeddyDD
Copy link
Contributor

TeddyDD commented May 30, 2015

This little mod appears in plenty of mod archives. What we are supposed to do with it?
I was looking for precedence but some metadata filters it out, other install it explicitly

https://github.com/KSP-CKAN/NetKAN/search?utf8=%E2%9C%93&q=MiniAVC

@Dazpoet
Copy link
Member

Dazpoet commented May 30, 2015

The .xml file should always be filtered. As for the .dll I am not sure how it functions. Could it even be bundled as a dependency or is it somehow bound to each mod?

@NecroBones
Copy link
Contributor

My understanding is that it's not "bound" as such. It's more like ModuleManager, in that it's used by a lot of mods, but not all of them bundle it. If multiple copies are installed, only the most recent copy of it will run.

@TeddyDD
Copy link
Contributor Author

TeddyDD commented May 31, 2015

So we should create package for dll and add it as dependency? Is this mod available on KS or as release on github?

@NecroBones
Copy link
Contributor

Looking at the forum thread: http://forum.kerbalspaceprogram.com/threads/79745

It looks like Mini AVC is a download on the mod author's website only. The main mod itself, "KSP AVC" is on curse and github, and is already in CKAN. I'm not sure what to recommend. Maybe the mod author should be contacted. It's possible he might recommend having the full mod listed as a suggested dependency or something.

@pjf
Copy link
Member

pjf commented May 31, 2015

Ooooh! I was hoping we'd have this discussion.

Users should be able to choose whether or not they wish to have MiniAVC or not. At the moment lots of folks end up with it anyway because of the way it's bundled with mods and we're aren't already filtering it out.

So, the ideal situation would be:

  • Our test processes generate a warning if they spot MiniAVC.dll being installed by a mod.
  • Mods which bundle MiniAVC.dll recommend it instead. Users doing a default install will still get it, just like they always have.
  • The user gets to make the choice if they want it or not.

This means folks who like MiniAVC can have it, and those who don't can choose not to have it, and everyone is happy. It also means that users are able to keep their MiniAVC library up to date (in much the same way that we do with ModuleManager).

@Dazpoet
Copy link
Member

Dazpoet commented May 31, 2015

Gonna be a real joy finding and fixing every mod that contains MiniAVC but I approve strongly of adding it as a recommend rather than a depend so I can finally have it not appear in my installs

@TeddyDD
Copy link
Contributor Author

TeddyDD commented May 31, 2015

I think we should ask MiniAVC author to create github releases for new MiniAVC so we could create .netkan for it. (I suppose we can't use http://ksp.cybutek.net/miniavc/ to create .netkan)

Contacting @CYBUTEK :)

@Dazpoet
Copy link
Member

Dazpoet commented May 31, 2015

Considering the last release was so long ago we could probably start by creating a .ckan file if nothing else. I assume most modders have moved over to the latest version by now.

@TeddyDD
Copy link
Contributor Author

TeddyDD commented May 31, 2015

ok, brb

EDIT:
Wait a second

The .xml file should always be filtered.

If we filer that xml files is this mod will works? O_O

@Dazpoet
Copy link
Member

Dazpoet commented May 31, 2015

the .xml files is created on running isn't it? I know we've had a case or 2 of mods refusing to upgrade properly because one version didn't bundle the .xml file while the next did and thereby collided with the generated .xml file from the previous version.

@Dazpoet
Copy link
Member

Dazpoet commented Jun 1, 2015

Hmm looking at http://ksp.cybutek.net/miniavc/Documents/README.htm it would appear the above merged PR might not be idea since the ReadMe states we should not install MiniAVC to GameData, or atleast that mods shouldn't.

For our purposes it would appear to be a very logical place to put it since it will check all subfolders of GameData for .version files and prompt the user for usage of AVC. We'll probably want the author to clarify how to handle this situation so that the plugin doesn't turn itself off if placed into GameData or somesuch.

@NecroBones
Copy link
Contributor

Something else to consider is that the full "KSP AVC" is a superset of "Mini AVC", and should probably count as meeting the dependency if its installed.

@pjf
Copy link
Member

pjf commented Jun 2, 2015

Catching instances where we're installing MiniAVC should be pretty straightforward, since we can get a bot to do it when we do netkan inflation. That same bot could even auto insert the recommendation/suggestion if the file was detected. :-)

None of this has to be part of netkan.exe proper, the indexer is already essentially a pipeline and this can be just an extra step.

@severedsolo
Copy link
Contributor

Is there even a need for MiniAVC if we are using CKAN?

All it's going to do is tell you "there is a new version available" - but then you have to wait for CKAN to catch up anyway. You may as well just let CKAN handle it.

Unless people use AVC to remind them to refresh their CKAN repo I suppose

@NecroBones
Copy link
Contributor

It's that latter case that I think is the important part. It lets people know something has changed if they haven't updated recently.

@mheguy
Copy link
Contributor

mheguy commented Jul 17, 2015

MiniAVC.dll creates its .xml in the folder in which it resides. The .xml only contains 2 pieces of information: 1) If MiniAVC should check for updates (which doesn't affect it warning about incompatible versions) 2) If it's the first time running.. So an .xml is not specific to a mod.

MiniAVC scans every .version file in directories below it. Meaning it will function from inside the GameData directory.

~~So I propose implementation in the following manner:

  1. Code the bots so that they filter MiniAVC.dll and MiniAVC.xml from all mods. (As proposed by @pjf)
  2. Create a metadata module (As proposed by @TeddyDD) for MiniAVC.dll and MiniAVC.xml to be installed. By including an .xml we can ensure that users won't have the file leftover if they choose to uninstall the mod.
  3. Get information about which mods include a .version file and add a suggests for MiniAVC based on that. (As proposed by @pjf)~~

While ideally all 3 steps would be completed at the same time, step 3 can be done significantly later than 1+2. Steps 1 and 2 should go hand in hand however, since a MiniAVC.dll in the GameData directory will create duplicate messages on launch for all mods that have a MiniAVC.dll of their own.

Ideally this should all be done to coincide with a release, so that the release notes can pass along information about the change.

Now I just need to get @techman83 and/or @dbent on board for the coding. 😄

Edit: See later comment

@NecroBones
Copy link
Contributor

That sounds like a pretty good solution to me.

@BobPalmer
Copy link

Ran into this by accident. I would say that pulling out miniAVC is pretty much against the express wishes of the modder who included/coded for it... i.e. it is there for a very good reason. It should be treated as a dependency no different than FireSpitter or Module Manager.

Transforming the modder's desire to how they bundle their mods from a dependency to a 'sugggest' is highly uncool.

@BobPalmer
Copy link

(An addendum) the 'optional' version of this is the full KSP-AVC, i.e. if a modder wishes to provide the option of support, we use KSP-AVC. If we depend on KSP-AVC to handle updates, release notes, pre-releases, etc. then we use MiniAVC.

politas added a commit to politas/NetKAN that referenced this issue Mar 18, 2016
Remove filtering of MiniAVC pending action and clear decision on KSP-CKAN#1455
@mheguy
Copy link
Contributor

mheguy commented May 5, 2016

We need to get out of managing miniavc. Currently we filter many mods' copies of miniavc.xml and that means we're not deleting the file when we uninstall the mod and that's not something we should do.

If a copy of miniavc.dll exists in both the GameData base folder and any subfolder then all dialogs surfaced by the mod are surfaced twice and that is annoying AF if you have a couple hundred mods installed. While CKAN can be coded to do that we can't be sure our users won't manually install a mod that has a copy of the dll. Because we can't/shouldn't restrict users from manual installations I don't think this can go forward as a concept.

Oh and installing to a subfolder means xml files won't be generated for other mods which means mods that rely on on-run generation won't be tested properly. Also not acceptable.

I've written up #3934 that will get our fingers out of miniavc files and close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants