Closed cake-pie closed 5 years ago
I just tried it, it is possible to index Harmony like every other mod hosted on GitHub. The version field is pulled from the release tag, yes.
If multiple mods install their own bundled copies of Harmony in the same KSP instance, will it work or not? If it works, then I'd rather not interfere with a mod's implementation details like that; we would have to filter this DLL out specifically from each mod that uses it.
Will need to experiment with that, concern is what happens if there are different versions, depending on load order might a mod end up getting an older Harmony than expected that lacks some feature etc.
I can report that when there are multiple copies, the newest version will be used.
Still, the situation seems analogous to Module Manager, I think.
If there are multiple copies of MM installed, they will elect one copy (of the newest version) to continue running, so it is safe for other mods to bundle a copy of it. MM includes the version number in the dll filename as well, to avoid the possibility of a newer version getting overwritten by an older one.
But in practice isn't it more common that MM doesn't get bundled, and is specified as a dependency in CKAN metadata? (For manual install, "batteries sold separately".) It leads to more consistency and a cleaner install for the end users too, rather than having disparate copies sprinkled all over GameData.
One thing that it is desirable to avoid is to get into a scenario where some end users start getting irritated at having redundant copies of the "same thing" and decide to do their own pruning, and then make the mistake of removing the newest copy.
If users are deleting files that CKAN itself installed, hopefully the risks of that would be pretty obvious. And there's a distinction with ModuleManager - it's usually installed in the GameData root, outside the mod's folder, whereas so far our one instance of 0Harmony.dll is located inside a mod's folder.
Also, consider manual installations. If a user installs 2+ mods that bundle 0Harmony.dll, by default they'll unpack the mod folders into GameData like any other mod. So any problems caused by having multiple copies of this DLL would already be something the affected mod authors would have to figure out.
@politas @Olympic1 @techman83 @pjf @dbent, how do you feel about indexing dependencies that are not KSP mods? This is a third party utility DLL that some mods may bundle (only one so far, but more may be on the horizon).
any problems caused by having multiple copies of this DLL would already be something the affected mod authors would have to figure out.
You are exactly right, affected mod authors will need to establish or evolve a set of best practices for how to bundle 0Harmony.dll in a way that avoids problems further down the line.
But that's not something that can be done in isolation. Mod authors would obviously prefer to release one package that can work for both CKAN and manual install, so what CKAN can/cannot / will/will not do plays a role in the decision making process here.
Which is why I'm reaching out in advance, to gather information and figure out next steps.
so far our one instance of 0Harmony.dll is located inside a mod's folder.
Sure, but I am not sure that that is the best approach. There are players that prune parts, players that delete all copies of mini-AVC.dll, etc.; as a mod author I would like to avoid support workload arising from someone doing DIY pruning of multiple 0Harmony.dll and then mucking it up.
If users are deleting files [...] the risks of that would be pretty obvious
But can be mitigated, whether it be CKAN or manual installed.
ModuleManager [is] usually installed in the GameData root
And that is kind of the model that I think would work well here -- as long as we follow MM's example and put the Harmony version into the dll filename.
So, my current preference is something like:
MyMod.zip
+ GameData
+ 0Harmony-1_2_0_1.dll
+ MyMod
+ etc
For manual installs, this gives players easy drag-and-drop but also
a) avoids older versions overwriting new ones, and
b) clearly identifies old versions that can be safely pruned
For CKAN, if I understand correctly it would be possible to specify
a) grab the MyMod folder and put it in GameData
b) also install latest 0Harmony.dll
But that requires indexing Harmony
CKAN could also choose to not index Harmony and just follow the same unpacking procedure as manual installs, but as someone who doesn't use CKAN as an end user I don't know what happens if ModA and ModB both install GameData\0Harmony-1_2_0_1.dll
and then for instance say ModB is uninstalled. Are there strong guarantees that the dll will be retained for ModA's use?
If a user tries to install two mods that both try to install GameData\0Harmony-1_2_0_1.dll
, then the second one will fail with a conflict. To install both mods, each would have to put its Harmony DLL in its own folder (or use a different filename, but this example illustrates pretty well the limitations of that).
Ah, I see, hence this specific guidance for shared dependencies
Sometimes dependencies are bundled with a mod for regular users. In these cases you should use an explicit
depends
relationship rather than getting the CKAN client to install bundled files.
how do you feel about indexing dependencies that are not KSP mods? This is a third party utility DLL that some mods may bundle (only one so far, but more may be on the horizon).
As someone who hasn't contributed in a while, I would be okay with it. Should probably establish some sort of convention for general purpose .NET libraries rather than KSP specific mods. Maybe a -DOTNET
suffix, a la the -DLC
suffix.
In that case, the general idea would be:
Harmony-DOTNET
.depends
relationship on Harmony-DOTNET
, optionally with version constraints.filter
or filter_regexp
properties or implicitly by not including it in the install
property.Is there a glaring issue with this approach?
Is there a glaring issue with this approach?
Not as such. My concerns are:
So I see moderate risks in exchange for no known benefit.
Harmony is wonderful, and I love it!
As far as I know it's fine for C# to load Harmony multiple times (if bundled with each mod), even if they're different versions. This is super common in the RimWorld modding scene. I've also never seen Harmony break backwards compatibility, so having One Library To Rule Them All also works.
Which means I have no strong technical opinions on this at all, and would be in favour of whatever results in the most harmony in the CKAN ecosystem. :)
Seems like CKAN ought to Do Nothing™ then and just recommend that mods distribute private copies of Harmony (which CKAN will install). If/when there is a conflict with multiple copies of Harmony, modders will have to figure out some mechanism to resolve said conflict and in the intervening time CKAN would add conflict
relationships between the affected mods.
Does that seem reasonable?
As far as I know it's fine for C# to load Harmony multiple times (if bundled with each mod), even if they're different versions. This is super common in the RimWorld modding scene.
I'd prefer to be a bit more cautious because Cities: Skylines modders appear to have had problems with multiple copies of Harmony bundled with different mods. See Harmony#102. Just because it works great in RimWorld may not guarantee the same for other games.
CKAN ought to Do Nothing™ then and just recommend that mods distribute private copies of Harmony
Except that individual mods aren't going to get their own "private copy" *. My testing shows that in the case of KSP, multiple mods bundling different versions Harmony would all end up using the same (newest) version, which can be coming from another mod's folder.
So let's say if I bundle Harmony 1.1 with ModA, there is no way for me to prevent someone else's ModB from bundling Harmony 1.2. ModA will then end up using 1.2. In the unlikely event that this leads to some conflict, it'd occur for both CKAN and manual installs and would be no fault of CKAN at all.
Since that is going to be the behavior in any case -- independent of whether CKAN or manual install -- what I'm advocating here is that CKAN and mod authors can cooperate to deliver the benefits of not cluttering the end user's disk with multiple copies of the same library, and eliminating any potential conflicts arising from having multiple copies.
If CKAN refuses to index Harmony, then the inability to use shared dependencies doesn't just "recommend" but in fact compels every mod that needs Harmony to bundle it inside their mod folder in order to support CKAN installs. Consuming more disk space and increased risks of end users doing amateur disk space cleanup, with zero difference in function.
(* Footnote: A mod could, if it really wants, build its own copy of Harmony in the mod's namespace and as part of the mod's own dll. But that's really excessive for something which most mods would just be using "off the shelf".)
So let's say if I bundle Harmony 1.1 with ModA, there is no way for me to prevent someone else's ModB from bundling Harmony 1.2. ModA will then end up using 1.2. In the unlikely event that this leads to some conflict, it'd occur for both CKAN and manual installs and would be no fault of CKAN at all.
Come think of it, CKAN could help ameliorate this.
ModA
depends
Harmony 1.0+.conflict
s Harmony 1.2ModB
depends
Harmony 1.0+.CKAN would be able to offer rollback to 1.1 pending resolution of ModA's issue with 1.2
In the interest of making progress on this...
Would it help if there was a KSP forum thread for the purposes of
None of this has anything to do with CKAN, really, but I'm willing to do it if needed. The hope is it alleviates some of the concerns I'm seeing from @HebaruSan re: if things break and CKAN gets fingered with the blame.
So, suppose some future ModX comes along and uses Harmony, and wants to be indexed on CKAN. The modder can be pointed to the thread for information. Now, if after being pointed to the available facts, the modder insists on having ModX bundle their own copy of the dll inside their mod folder instead of using the CKAN-indexed Harmony, that is their prerogative.
CKAN would not be obliged to actively filter out Harmony from ModX's install; there is no need to "interfere with a mod's implementation details", and CKAN can still install ModX correctly according to the modder's wishes.
In that case, if ModX is installed alone, it is guaranteed to work as the modder intended.
If it is installed in the presence of other mods that do used the CKAN-indexed Harmony, or also bundle their own copy of Harmony, then things should still work as long as no issues arise from having multiple copies of Harmony. (So far seems generally safe based on my testing.)
However, if problems do arise due to multiple copies of Harmony-- which would applies equally for manual and CKAN installs -- then it's really ModX's fault that they refused to coordinate with other Harmony-using mods. It has nothing to do with CKAN at all.
As @DasSkelett has reported, indexing does work and I tried it myself with the following netkan which should correctly locate the .NET 3.5 version of the dll from the Release.zip github asset (the other being Debug.zip).
{
"spec_version" : "v1.16",
"identifier" : "Harmony-DOTNET",
"name" : "Harmony",
"abstract" : "A library for patching, replacing and decorating .NET and Mono methods during runtime.",
"author" : "Andreas Pardeike",
"license" : "MIT",
"$kref" : "#/ckan/github/pardeike/Harmony/asset_match/[Rr]elease.zip",
"ksp_version" : "any",
"resources": {
"homepage": "http://forum.kerbalspaceprogram.com/index.php?showtopic=000000",
"repository": "https://github.com/pardeike/Harmony"
},
"install": [
{
"find": "net35/0Harmony.dll",
"find_matches_files": true,
"install_to": "GameData"
}
]
}
Would even better if we can rename the file to include version number e.g. 0Harmony.1.2.0.1.dll
but I'm not sure if the as
directive for install
supports that?
This issue is now a blocker that is preventing full release of the next version of AirlockPlus.
I withheld my reply for a few days, waiting for a response by another CKAN team member, as I'm obviously the newbie here, but I guess I can wait a long time.
I'm mostly on @cake-pie's side. First where I disagree: Indexing non-mods is a pretty big change, I think. That might be well thought out, and you can take two months or more to get a definitive answer. We do that all in our free time at least. Maybe you shouldn't rely on a feature, that perhaps might be introduced in the (potential far) future, that isn't even fully discussed and layed out yet, speaking in general programmer terms.
We however should've kept in touch. We shouldn't have gone silent suddenly, not answering repeated questions in any form, turned deaf (and yes, I'm guilty of that, too). If you need time to find an answer, you could just say so. A simple "Sorry, we can't say that yet, go on without us meanwhile, we'll get back to you if there's anything new" or something similar would be enough. And someone would have had time for that.
@cake-pie mentioned that his next release depends on it. We just let him down. We could have just said that he should continue like before, distributing Harmony himself, and everything would be fine. But no answer is the worst answer. That is not fair and no good behavior.
To get to the point of this issue: I am pro indexing of Harmony, the arguments of @cake-pie convinced me. Probably we will only get a final answer through a vote?
Hmm based on the comments by @pjf and @dbent, is the status quo not ok? I'm generally in favour of path of least changes. Do we have a lot of mods which bundle harmony? Is this causing considerable problems?
Well yeah, it will work. And right now, I only know one mod bundling Harmony right now. I'm fine if we don't index it (now), if the opinion of the team // the need for indexing it changes one day, it should be possible to do that then.
So I would advise @cake-pie now to redo his planned release and rebundle Harmony (which I'm kinda doing right now in the case he's reading along :P). Is that okay?
Again, as someone who hasn't actively contributed in a very long time, the status quo seems fine to me. If/when it becomes a problem, consider what the solution would be if CKAN did not exist; that's the ultimate solution. CKAN may help alleviate or illuminate certain problems, but it shouldn't be relied upon to fix them.
Is this causing considerable problems?
It could, and I'd rather not find out the painful way when there exists a simple, obvious, straightforward solution that would avoid any problems altogether.
consider what the solution would be if CKAN did not exist; that's the ultimate solution.
Modders would need to come to a consensus on best practices of how to bundle Harmony so that multiple mods will share the same Harmony dependency, instead of having multiple, separate, conflicting copies.
Which is exactly what I'm asking for.
It is also what your own documentation prescribes:
you should use an explicit depends relationship rather than getting the CKAN client to install bundled files.
By refusing this, CKAN compels mods to include their own copy of the dependency, and drives the situation towards any issues and conflicts that exist.
path of least changes
If/when it becomes a problem
So, inertia prevents taking a proactive approach to preempt and avoid perfectly preventable issues, and you'd rather wait for problems to crop up?
CKAN [...] shouldn't be relied upon to fix [problems].
CKAN shouldn't cause problems.
I am trying to avoid problems. CKAN is forcing me in a direction that leads towards problems that are avoidable.
There is a safe route here. Why insist on taking the path that would allow conflicts to manifest? And by extension, force mod authors down this road together with you?
I've already debunked concerns re: CKAN having to meddle with the installation of mods. If there are any other real concerns, feel free to raise them.
Moot as of #7220.
Note that as of now, these are the mods that I know of bundling Harmony :
I agree that having CKAN actually being a provider for the harmony DLL is unnecessary.
However, there is definitely a use case for Harmony being indexed. Harmony 2.x is incompatible with Harmony 1.x, and side-loading the two versions doesn't work and will cause mods to hard-crash. That issue has caused quite a mess in other modding communities where Harmony is much more popular. Currently, all the mods I listed are still using Harmony 1.x, but I am planning to update Kerbalism to Harmony 2.x at some point in the future.
So it would be nice if CKAN was indexing harmony as a purely "virtual" dependency, as that would give us a way to manage that incompatibility, at least for CKAN installs. I agree it doesn't solve the core issue in any way, but that would be an additional safeguard.
I mentioned this earlier in https://github.com/KSP-CKAN/NetKAN/pull/6792#issuecomment-482929482
This is now actually becoming an issue because I also found myself using it as a dependency for a WIP mod ( forum | github ).
Harmony is a patching library for .NET in general, and not a KSP mod in the usual sense, so I don't think it's appropriate to ask them to put a *.version file somewhere. On the other hand we probably don't have to worry about its compatibility with different versions of KSP. And any KSP mods that use it as a dependency will probably be fine with using the latest version (unless something gets deprecated or there is some kind of regression.)
Is there anything clever that can be done on CKAN side of things to grab the version number from the git tag of the release, or the dll assembly info?