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

Add the Harmony lib as a package #8288

Merged
merged 6 commits into from Mar 17, 2021
Merged

Add the Harmony lib as a package #8288

merged 6 commits into from Mar 17, 2021

Conversation

sarbian
Copy link
Contributor

@sarbian sarbian commented Dec 28, 2020

Initial description

This is related to #7131, #7220 and #7200.

@gotmachine has plan to use Harmony version 2.x and I will do the same for some mods that are becoming to hard to maintain with only the stock API and old tricks (namely ModularFlightIntegrator and CustomBarnKit).

My point of view is that having each mods package the DLL is a recipe for a disaster. We already know that Harmony 1.x and 2.x are not compatible and the the current KSP dll loader is a lot stupider since the upgrade to Unity 2019 which made having multiple versions of the dll a mess (Multiple version of MM does not work all that well since that upgrade).

So I want to add Harmony as a CKAN deps and encourage mods to not bundle it. It would also require that the few mods that already use it make the upgrade. It does not look complex but it still requires a new build.

@gotmachine @LunaMultiplayer @Kerbalism @drewcassidy : You are the authors that use and bundle Harmony 1.x. Would you agree to upgrade to 2.x and keep it out of your zip ? If possible we could agree to release new version that uses 2.x around a specific date to avoid breaking things.

Sorry to use this repo to talk about this but that subject would be taboo on the forum. We can use on of my repo to talk about it if needed.

Updates

@HebaruSan
Copy link
Member

HebaruSan commented Dec 28, 2020

a recipe for a disaster. We already know that Harmony 1.x and 2.x are not compatible and the the current KSP dll loader is a lot stupider since the upgrade to Unity 2019 which made having multiple versions of the dll a mess

Frankly the more I hear about Harmony the more it sounds like using it at all is a recipe for disaster. Mods depend on a specific version of Harmony, and different versions of Harmony are incompatible with one another? That's not going to work out well for anyone regardless of what we do in CKAN.

Sorry to use this repo to talk about this but that subject would be taboo on the forum

Personally I'm fine with it if mod authors want to coordinate here.

@sarbian
Copy link
Contributor Author

sarbian commented Dec 28, 2020

Frankly the more I hear about Harmony the more it sounds like using it is a recipe for disaster. Mods depend on a specific version of Harmony, and different versions of Harmony are incompatible with one another? That's not going to work out well for anyone regardless of what we do in CKAN.

Only 1.x and 2.x are incompatible. You can have multiple version of 2.x or 1.x but not both. The author cleaned the ABI and used a different namespace but it was not enough to avoid problems. I do not remember the specifics but I think it is related to the Unity Mono.
If everyone move to 2.x we will be fine.

@HebaruSan
Copy link
Member

Only 1.x and 2.x are incompatible

That's bad enough already though (to say nothing of what it says about how future 3.x, 4.x, etc. versions will be handled). Some user finds a mod that looks neat, it says it's compatible with their KSP version, they install it, but it has a hidden conflict with one of their other mods because they rely on different versions of Harmony? Not great.

@HebaruSan
Copy link
Member

So...

  • What would this look like in practice, in terms of the specific metadata that would be used for Harm-dependent mods?
  • How are we imagining this will help manage the compatibility problems?
  • What will the user experience be like during the transition phase between Harm1 and Harm2?
    • What happens to a user who has a mod installed that depends on Harm1, who then wants to install the future MFI with Harm2?
  • Who will be responsible for monitoring which mods are compatible with Harm1 and which are compatible with Harm2, and updating the metadata when those lists change?
  • Is AirlockPlus still being maintained?

@blowfishpro
Copy link
Contributor

to say nothing of what it says about how future 3.x, 4.x, etc. versions will be handled

I don't think breaking changes, and therefore major version bumps, are going to be too common. It was more than 3 years between 1.0 and 2.0. I would actually expect the rate of breaking changes to fall off as it matures.

@gotmachine
Copy link
Contributor

gotmachine commented Dec 28, 2020

I have no problem with moving Kerbalism to harmony 2.x (in fact I almost did for the last release and decided not to because of the potential compatibility issues).

However, manual (non CKAN) mod distribution mean that users will be responsible for where they should copy the right harmony dll. Given how the distributed harmony zip is packaged (with the multiple .Net targets in subfolders) and how hard it is for the average player to install things the right way even when this is dead easy with clear instructions, I'm not optimistic.

IMO, the right way to handle the situation would be to create a github organization and repo that redistribute the net472 harmony dll in a standardized GameData/00Harmony folder alongside a .version file, maintain the ckan metadata and eventually also publish it on spacedock/curse. That organization could have a few trusted members amongst a few of us to ensure it is properly maintained. Eventually, it could also bundle a lightweight plugin that check the loaded assemblies and warn users in case several versions / instances are found.

Harmony is a quite mature product that is very widely used. Given the drama that happened due to the v1 > v2 transition in other modding scenes where harmony is one of the primary modding tools, it is likely that we won't see that issue happen again anytime soon and that the harmony maintainer will take extra care in regard to backward compatibility.

Per my comment in #7131 (comment), there are only a handful of mods currently bundling harmony. I believe AirlockPlus and Tilt-Em are more or less abandoned. At the current point, it is possible to handle the situation with some coordination, but for some reason Harmony is becoming suddenly popular in the KSP modding scene (I think the people working on KSPIE are also thinking about using it) and the more we wait, the messier it will get. Migrating a mod from harmony v1 to v2 is trivial, and Harmony v1 is unmaintained and will become obsolete, so there is no reason that I'm aware of for any of us to still use it, other than the coordinated migration issue.

@HebaruSan
Copy link
Member

HebaruSan commented Dec 28, 2020

Brainstorming a bit, what about the harsh approach of declaring CKAN, as of some date, a Harm2-only zone, based on the problems that other modding communities saw from it?

We could freeze all mods that bundle Harm1, then start indexing ones that use Harm2 (whether via bundling or as its own CKAN module, doesn't seem like it would make much of a difference). This would have the advantage of providing a clear rule of thumb for new mods regarding what version the playerbase is using.

We would still have to figure out how to get existing users to uninstall Harm1-based mods. Maybe we could mark them all as conflicting with the Harm2 module?

@gotmachine
Copy link
Contributor

gotmachine commented Jan 9, 2021

Took the liberty to move this forward a bit.
Created an organization and a repository, invited @HebaruSan @sarbian @blowfishpro and @taniwha as extra owners.
I also added a basic version/compatibility checker plugin, seems to work well enough but if someone want to review/change it, do it. looks like this :
image

@HebaruSan
Copy link
Member

HebaruSan commented Jan 9, 2021

Thanks for the invite, but since I won't be using Harmony in my mods, I'm not interested in spending time trying to clean up after it.

I'm happy to help on the CKAN side of things, though.

EDIT: Oops, left out a very important "not"!

@gotmachine
Copy link
Contributor

Yep I understand, I added you just in case you have to intervene on the distribution side of things. And also because between all five of us, that ensure there will be at least one around to maintain it or transfer the ownership to someone else if necessary.

@DasSkelett
Copy link
Member

Brainstorming a bit, what about the harsh approach of declaring CKAN, as of some date, a Harm2-only zone, based on the problems that other modding communities saw from it?

We could freeze all mods that bundle Harm1, then start indexing ones that use Harm2 (whether via bundling or as its own CKAN module, doesn't seem like it would make much of a difference). This would have the advantage of providing a clear rule of thumb for new mods regarding what version the playerbase is using.

We would still have to figure out how to get existing users to uninstall Harm1-based mods. Maybe we could mark them all as conflicting with the Harm2 module?

Leaving some additional thoughts I had on how to handle it on the CKAN side:

We index the current Harmony as Harmony2, conflicting with a virtual Harmony1 module.

This way we don't have to freeze mods and mod versions bundling Harmony1, instead we could make them provide the virtual Harmony1 module and our relationships logic will make sure that users only got mods with one or the other installed at the same time.

This may lead to situations where users have to choose between one mod or the other if there's a mod not yet migrated to Harmony2, but better they do that in CKAN before installing, than finding out when launching KSP.

@HebaruSan HebaruSan mentioned this pull request Jan 31, 2021
@HebaruSan
Copy link
Member

Coming back to this, @gotmachine, if you have plans to create a release in the new repo, we could update this PR's netkan to reference it and start moving on @DasSkelett's idea about how to handle the conflicts (I'm thinking a new commit that adds "provides": [ "Harmony1" ], to the mods currently using Harmony1).

We index the current Harmony as Harmony2, conflicting with a virtual Harmony1 module.

I wonder if we could future-proof a bit more (i.e., potentially accommodate Harmony3, Harmony4, etc.) by using the Harmony identifier for both? If the main module conflicted with itself, and then any mod bundling it had the "provides" property set, that should have the same outcome of preventing incompatible installs without painting ourselves into a corner with regard to the versioning, right? (It would be unfortunate to eventually have a release of Harmony 3 indexed under a Harmony2 identifier because of a decision we made about this one transition.) I may have to test this out...

@DasSkelett
Copy link
Member

I wonder if we could future-proof a bit more (i.e., potentially accommodate Harmony3, Harmony4, etc.) by using the Harmony identifier for both? If the main module conflicted with itself, and then any mod bundling it had the "provides" property set, that should have the same outcome of preventing incompatible installs without painting ourselves into a corner with regard to the versioning, right? (It would be unfortunate to eventually have a release of Harmony 3 indexed under a Harmony2 identifier because of a decision we made about this one transition.) I may have to test this out...

Thinking through a bunch of scenarios, this should work, too. The only issue I see is that we can not easily spot the Harmony version in the metadata. At the beginning it would be implied that if it's a depends, it's Harmony2, if it's a provides, it's Harmony1. Make one or two years go past, or just someone new looking at the metadata, it'll be very confusing. Someone might swap out the provides with a depends (and a filter) because in theory it makes sense, but they don't know that it's different Harmony versions.
We should at least add a comment in each module in this case.

@gotmachine
Copy link
Contributor

I'm thinking a new commit that adds "provides": [ "Harmony1" ], to the mods currently using Harmony1

That seems like a good idea.

The only issue I see is that we can not easily spot the Harmony version in the metadata. At the beginning it would be implied that if it's a depends, it's Harmony2, if it's a provides, it's Harmony1. Make one or two years go past, or just someone new looking at the metadata, it'll be very confusing. Someone might swap out the provides with a depends (and a filter) because in theory it makes sense, but they don't know that it's different Harmony versions.

That indeed seem potentially confusing. And in the (unlikely) scenario where a Harmony 3 is released, we can decide to index it under Harmony2 if there is backward compatibility, or under Harmony3 otherwise. But frankly, from what I know of the state of the Harmony project, I don't expect that scenario to happen.

On the state of the KSPHarmony repository, I will try to move things forward this week, do a release and also prepare a Kerbalism release to use it when ready. With that KSPW00tNow mod release bundling harmony 2, we are already in a bad situation anyway. Also, I won't be around to carry the torch in the next weeks/months, so @sarbian / @blowfishpro / @taniwha / @drewcassidy : feel free to move things forward on your own.

@HebaruSan
Copy link
Member

HebaruSan commented Mar 16, 2021

With that KSPW00tNow mod release bundling harmony 2, we are already in a bad situation anyway.

Oh, how can you tell? I thought it was Harmony1 since it was the same filename. Maybe we should pull that mod until we get this sorted.

EDIT: Aha, the csproj has a version:

    <Reference Include="0Harmony, Version=2.0.4.0, Culture=neutral, processorArchitecture=MSIL">
      <HintPath>..\packages\Lib.Harmony.2.0.4\lib\net472\0Harmony.dll</HintPath>
    </Reference>

@gotmachine
Copy link
Contributor

Oh, how can you tell?

Checked the AssemblyVersion of the bundled 0Harmony.dll file (you can see that in IlSpy or with a windows explorer right click > Properties > Details)

@HebaruSan
Copy link
Member

HebaruSan commented Mar 16, 2021

OK, KSPW00tNow is now removed from CKAN temporarily. We can re-add it along with the other Harmony-related fixes in this PR.

@HebaruSan
Copy link
Member

HebaruSan commented Mar 17, 2021

Pushed some changes here to use the new repo, add the conflict with current bundlers, and re-add KSPW00tNow with depends and filter. The checks will fail until the new repo has a release.

(We should also do a CKAN-meta PR to add "provides": [ "Harmony1" ], to the old versions of those mods. Otherwise users might still end up with both installed via historical versions. But that can wait till after this is merged.)

@HebaruSan
Copy link
Member

HebaruSan commented Mar 17, 2021

I also added a basic version/compatibility checker plugin, seems to work well enough but if someone want to review/change it, do it. looks like this :
image

@gotmachine, will this plugin run on all KSP versions? Right now Harmony2.netkan has "ksp_version": "any", and we should make sure that's true of everything it installs.

EDIT: Ahh, you've got a version file. Good, that will simplify things...

@HebaruSan
Copy link
Member

Thanks for the help everyone, I think we are ready to dive off of this cliff...

@HebaruSan HebaruSan merged commit efef374 into KSP-CKAN:master Mar 17, 2021
@HebaruSan
Copy link
Member

image

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

Successfully merging this pull request may close these issues.

None yet

5 participants