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

KSP2 support #3797

Merged
merged 2 commits into from
Mar 11, 2023
Merged

KSP2 support #3797

merged 2 commits into from
Mar 11, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 28, 2023

Motivation

Kerbal Space Program 2 is now available in early access!

While debate rages over which game is better to play right now, the community is sure to shift to the shiny new toy over time unless something catastrophic happens, and mods are already starting to roll out for the sequel. The people want CKAN!

History

#3223 and #3308 abstracted all game-specific behavior behind the IGame interface, which now finally pays off!

Changes

  • Added KerbalSpaceProgram2 class reflecting how the new game works
    • Instead of selecting buildID.txt to add an instance, you just pick the KSP2_x64.exe

    • Instead of getting the game version from buildID.txt, we retrieve it from the EXE's properties
      image

    • GameData is replaced by the folder for the (now main/only) third party mod loader, BepInEx/plugins

  • Made Netkan multi-game-aware with a new --game KSP / --game KSP2 command line flag to control which game's rules are used for inflation
  • Netkan also now looks for files called swinfo.json and parses them for juicy tasty metadata according to the SpaceWarp format
  • The Inflator's Dockerfile now passes --game $GAME so the Infra can tell it which game to use

Metadata repos are already created and populated with some test data:

I have not acquired author permission for the mods currently up there (except BepInEx, see KSP-CKAN/NetKAN#9584, and SpaceWarp, see comment below), so we'll need to either get permission or purge them before merge.

image

image

Fixes #2863.

@HebaruSan HebaruSan added Enhancement Core (ckan.dll) Issues affecting the core part of CKAN Pull request Spec Issues affecting the spec Netkan Issues affecting the netkan data Schema Issues affecting the schema labels Feb 28, 2023
@HebaruSan

This comment was marked as resolved.

@HebaruSan

This comment was marked as outdated.

@techman83
Copy link
Member

techman83 commented Mar 1, 2023

  • NetKAN-Infra needs to be updated to run the Scheduler, Inflator, Indexer, and Mirrorer for the new repos

Yep, we can come up with a plan for that

  • What about the status page? Combine all mods into one list?

I'd love for this to be re-factored in some way to get us HTTPs

  • We're going to need new queues, too, I think. The KSP2 scheduler's messages need to go only to the KSP2 Inflator, not the KSP1 inflator. Same for the queue between the Inflator and the Indexer.

We can probably keep the same queues, but maybe we can put a game in a message attribute and have subscriptions that filter on that attribute. It's possible we could even just re-use most of the same code and just have it switch depending on the game.

@HebaruSan
Copy link
Member Author

Permission acquired from SpaceWarp author in SpaceDock Discord:

image

So now we don't have to worry about purging that one before merging.

@HebaruSan

This comment was marked as resolved.

Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments from my initial read through. I'm leaning towards not duplicating our Infra to support a second game, as it's a bunch of stuff that'll be largely idle and we can do a little work for it to fit into the existing pieces. That way if we reach scaling issues we can just increase the number of inflators (it's the service that works the hardest)

Core/Games/KerbalSpaceProgram2.cs Outdated Show resolved Hide resolved
Dockerfile.netkan Show resolved Hide resolved
@HebaruSan
Copy link
Member Author

HebaruSan commented Mar 3, 2023

I'm leaning towards not duplicating our Infra to support a second game, as it's a bunch of stuff that'll be largely idle and we can do a little work for it to fit into the existing pieces.

What if we reduce the memory/disk requirements of the new containers for KSP2, since there will be very little metadata cloned from very small repos initially (and probably small mods, to boot, while the game is too unplayably buggy to create anything large)? I'm concerned that updating several services to clone multiple (pairs of! for NetKAN and CKAN-meta) repos and multiplex requests among them would be much larger than just a little work, as well as creating many many opportunities for new bugs.

Would 'memory': '64' be enough, do you think?

That way if we reach scaling issues we can just increase the number of inflators (it's the service that works the hardest)

That advantage is shared by both approaches, and in fact can be applied with more granularity with re-use of the containers: If we reach scaling issues, we could just increase the number of KSP1 inflators, or increase the number of KSP2 inflators, or both, as needed.

@HebaruSan
Copy link
Member Author

I'm not enthusiastic about encoding attributes of the outer multi-game reality into code. The way they are now, each service knows about its own repos, queues, etc., and doesn't need to care about the high level view of how many games there are, what they're called, what repos they use, etc. I think that's great! We made these tools that do one thing and do it very well, and we already have a way to deploy them in slightly new ways.

@techman83
Copy link
Member

I'm not enthusiastic about encoding attributes of the outer multi-game reality into code. The way they are now, each service knows about its own repos, queues, etc., and doesn't need to care about the high level view of how many games there are, what they're called, what repos they use, etc. I think that's great! We made these tools that do one thing and do it very well, and we already have a way to deploy them in slightly new ways.

I will code up an approach, but from a perspective of all the services, there is no difference in logic beyond what repo to push the metadata into. Running an extra set of services just to differentiate a game seems quite wasteful. The scheduler can probably be configured separately though.

It would be nice to consider a future of further game support, though well out of scope here, it doesn't seem like a significant amount of work to generalise the infra.

@HebaruSan
Copy link
Member Author

a bunch of stuff that'll be largely idle

We could always schedule KSP2 inflation passes every 5 minutes, if the idleness bothers you... 😁

@techman83
Copy link
Member

a bunch of stuff that'll be largely idle

We could always schedule KSP2 inflation passes every 5 minutes, if the idleness bothers you... grin

Hahaha, not so much the idleness. It's more I can see in my mind a pretty trivial way to do a fairly small set of changes and it would cover an infinite number of games.

@HebaruSan HebaruSan force-pushed the feature/ksp2-support branch 2 times, most recently from a527722 to 4453410 Compare March 7, 2023 23:07
@HebaruSan
Copy link
Member Author

Just updated:

  • SpaceWarp is now just a BepInEx plugin rather than a loader and no longer has its own special folders, so those folders are removed from KerbalSpaceProgram2.cs (note that this will mean the existing test data won't install anymore and will have to be replaced)
  • SpaceWarp's metadata file is renamed from modinfo.json (expected to be used by the official loader and thus a potential conflict) to swinfo.json, so Netkan now looks for that name as well (and the name "ModInfo" in code is now changed to "SpaceWarpInfo")
    (They also fixed my complaints about the syntax of the example file not being valid! 🎉 )

@HebaruSan HebaruSan force-pushed the feature/ksp2-support branch 2 times, most recently from ee619db to a81daf9 Compare March 8, 2023 00:44
@HebaruSan
Copy link
Member Author

HebaruSan commented Mar 8, 2023

A few more things:

  • Replaced "KSP" with "game" in schema descriptions
  • Removed SpaceWarp/Mods from schema
  • Made netkan.exe --verbose much less noisy
  • Make the swinfo.json handler depend on $vref: '#/ckan/space-warp' so we can turn it off in the case of bundled mods or specify the path (including spec update and making VrefValidator not confuse this with a ksp-avc vref)
  • Use ModuleService.ApplyVersions to merge compatibility nicely a la KSP-AVC and avoid conflicting properties
  • The new transformer was duplicating the module into one that included the swinfo metadata and one that didn't, and then the latter overwrote the former; fixed with yield break

Also just pushed updates to KSP2-NetKAN and KSP2-CKAN-meta to make the latest releases inflate cleanly.

@HebaruSan
Copy link
Member Author

HebaruSan commented Mar 9, 2023

  • Renamed builds.json embedded resource to builds-ksp.json
  • Added builds-ksp2.json embedded resource, which is just an array instead of a map because there are no build IDs for the new game
  • Created https://github.com/KSP-CKAN/KSP2-CKAN-meta/blob/main/builds.json
  • Added IGame.RefreshBuilds
  • Implemented KerbalSpaceProgram.RefreshBuilds calling ServiceLocator.Container.Resolve<IKspBuildMap>().Refresh();
  • Implemented KerbalSpaceProgram2.RefreshBuilds to do basically the same thing more simply for the new game's build list

@HebaruSan
Copy link
Member Author

HebaruSan commented Mar 9, 2023

  • The Inflator's outgoing messages now have a GameId attribute with values either KSP or KSP2

Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks reasonable from my read through, and an E2E test with the attribute changed to GameId, all looked good.

I'll leave it up to your discretion @HebaruSan - but I think the attribute name change would make sense in the context of an SQS message.

Netkan/Processors/QueueHandler.cs Outdated Show resolved Hide resolved
Co-authored-by: Leon Wright <techman83@gmail.com>
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 Netkan Issues affecting the netkan data Pull request Schema Issues affecting the schema Spec Issues affecting the spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] KSP2 support plans
2 participants