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 G.R.A.P.E.F.R.U.I.T from SpaceDock #9855

Merged
merged 5 commits into from
Nov 22, 2023
Merged

Add G.R.A.P.E.F.R.U.I.T from SpaceDock #9855

merged 5 commits into from
Nov 22, 2023

Conversation

netkan-bot
Copy link
Member

This pull request was automatically generated by SpaceDock on behalf of Dawn0303, to add G.R.A.P.E.F.R.U.I.T to CKAN.

Please direct questions about this pull request to Dawn0303.

Mod details

Key Value
Name G.R.A.P.E.F.R.U.I.T
Authors Dawn0303
Abstract [WIP] Adds parts that allow you to convert empty tanks into luxury habitats
License CC-BY-NC-SA
Size 68.55 MiB
Homepage https://forum.kerbalspaceprogram.com/topic/220714-wip-grapefruit-formerlytankhabs-help-wanted/
Source code https://github.com/dawn0303/GRAPEFRUIT

Description

Gateway Residence for Astronauts Providing Enhanced Facilities for Researching and Understanding Interstellar Travel
[HELP WANTED]

This mod is designed to add parts that allow you to convert empty tanks into luxury habitats, heavily inspired by the Space Shuttle External Tank Station concept

this mod is still heavily WIP, you can find the roadmap Here

currently looking for someone who can help me make a module for the parts as i lack the ability right now, message me on TWT or the ksp forums if youre interested in helping!





This is an automated commit on behalf of Dawn0303
@JonnyOThan
Copy link
Contributor

JonnyOThan commented Nov 22, 2023

So this one is interesting because it bundles the HabUtils dll from SSPX. I don't know if this is a long-term approach, and I haven't investigated how it's being used yet. From talking to the author they are very concerned with limiting the dependencies of this mod, so we probably should not make SSPX a dependency here (but of course we could if there are no other solutions).

One solution would be to make HabUtils its own package in CKAN, and have GRAPEFRUIT and SSPX depend on it. Not sure if we'd need @ChrisAdderley's permission for that.

@dawn0303 could you provide some background info on how you're using HabUtils so we can make some informed decisions here?

@dawn0303
Copy link
Contributor

Basically, the mod just adds inflatable habs, and they all use the HabUtils to inflate, as the base game moduleAnimateGeneric was causing issues for others testing the mod, all of which HabUtils fixes.

the idea with packaging the DLL alone is that I didn't want to force people to have to also install all of SSPXr if they wanted to use my mod, and as the DLL is on the MIT licence I thought it would be ok.

it wasn't originally planned as a long-term approach as I would like my mod to have an original module, but I'm not certain if that will ever happen as it's something I would need a lot of help with.

if dependency is the only possible option I would be ok with that, but having the DLL be its own package would not only help this mod, but could be very helpful for future first-time modders

@JonnyOThan
Copy link
Contributor

@dawn0303 are the problems you mentioned with ModuleAnimateGeneric limited to the IVAs not being created properly? I believe those are fixable in KSPCF (but of course that would add another dependency...), and you'd be waiting on whenever that patch gets written (KSPModdingLibs/KSPCommunityFixes#169). I suppose there's probably other benefits of using habutils as well (better integration with life support mods probably).

@dawn0303
Copy link
Contributor

yeah HabUtils is just a lot more versatile than even the fixed base game module, and life support mod integration is already in the works from some contributors

@JonnyOThan
Copy link
Contributor

Trying to collect the set of options here. This is further complicated by the fact that SSPX manages its own metanetkan files.

  1. Make GRAPEFRUIT depend on SSPX. No metadata changes needed, simple for users, but introduces a big dependency that isn't strictly required. Doesn't require any input from nertea.
  2. Make a new HabUtils mod in CKAN, derived from the SSPX package. Make SSPX provide HabUtils and conflict with it (so you can only have one installed). This satisfies Dawn's desires to limit dependencies and doesn't require any input from nertea. It's a little more complicated for users, because if they have the standalone HabUtils and then want to install SSPX it will show up as a conflict. Users would also have to download the entire SSPX package just for the HabUtils dll (although it wouldn't be installed).
  3. Same as option 2 except extract the HabUtils dll from the GRAPEFRUIT package or some new repository/release. Same benefits and drawbacks as option 2 and users won't have to download as much. However if the HabUtils in SSPX is ever updated, this package would also need to be. Unlikely in this case, but you never know.
  4. Ask Nertea for permission to take ownership of SSPX's metadata into the netkan repository (or send him PRs for the metadata that he'd have to merge). Then make a new HabUtils netkan that extracts the dll from SSPX and make SSPX depend on it. This is cleaner from the user's point of view, though they'd still be downloading a lot just for this dll. Downside: requires nertea's involvement
  5. Same as option 4 but fork HabUtils into its own repository (as in option 3). This addresses the download size concern. It's more complex for nertea if he ever wants to update HabUtils.

Did I miss anything? Thoughts?

@dawn0303
Copy link
Contributor

those seem like the best options yes, id say out of the list, 3 would make the most sense if nertea says no/doesnt respond, and 5 if nertea agrees

@HebaruSan
Copy link
Member

HebaruSan commented Nov 22, 2023

  1. Install everything from this mod for this module and make it conflict with SSPX
  2. As 6, but test whether a conflict is actually needed, maybe it's safe to have both DLLs?

(I'm not a fan of either of these per se, just adding for completeness.)

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Nov 22, 2023

Yep, didn't think of that one (6). simple, but pretty crappy for users.

@dawn0303
Copy link
Contributor

i was told both dlls would create issues, and I would prefer if it was playable alongside SSPXr, a hard dependently is preferable to a hard conflict

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Nov 22, 2023

@dawn0303 would you mind investigating what happens if you have both? I do think ksp 1.12.2 or so introduced some protections against multiple DLL installs. So that might be an option if you limit compatibility with those versions.

There is a potential issue there if HabUtils is ever updated - presumably KSP will pick one of the two DLLs to load and ignore the other. I'm assuming that since GRAPEFRUIT is alphabetically earlier than SSPX, it will be grapefruit's version. But that's a pretty unlikely situation. And maybe we could address it by installing HabUtils into a different folder.

@dawn0303
Copy link
Contributor

will do!

@dawn0303
Copy link
Contributor

KSP.log
nevermind, from the looks of things there isn't an issue, maybe good to look over in case I missed something, but it seems to work fine, so may just bundle in the mainfolder

@HebaruSan
Copy link
Member

Looks like NearFutureProps is also a dependency? I'll go ahead and add that, let us know if it shouldn't be.

@dawn0303
Copy link
Contributor

it definitely should be

@JonnyOThan
Copy link
Contributor

Very interesting. The AssemblyLoader says that it loaded twice, and there's no extra output regarding duplicated DLLs or anything. But the MM assembly listing and the shabby patching records seem to indicate that there's only one instance loaded.

[LOG 18:34:55.849] Load(Assembly): SSPXrModuleOnly/Plugins/HabUtils
[LOG 18:34:55.849] AssemblyLoader: Loading assembly at E:\SteamLibrary\steamapps\steamapps\common\Kerbal Space Program\GameData\SSPXrModuleOnly\Plugins\HabUtils.dll
[LOG 18:34:55.850] Load(Assembly): StationPartsExpansionRedux/Plugins/HabUtils
[LOG 18:34:55.850] AssemblyLoader: Loading assembly at E:\SteamLibrary\steamapps\steamapps\common\Kerbal Space Program\GameData\StationPartsExpansionRedux\Plugins\HabUtils.dll
HabUtils v1.0.0.0
[LOG 18:35:08.101] [Shabby] Patching call site : HabUtils::VisualDebugUtils.DebugAxisTripod.CreateBasicRenderer
[LOG 18:35:08.104] [Shabby] Patching call site : HabUtils::VisualDebugUtils.DebugPoint.CreateBasicRenderer
[LOG 18:35:08.105] [Shabby] Patching call site : HabUtils::VisualDebugUtils.DebugLine.CreateBasicRenderer
[LOG 18:35:08.107] [Shabby] Patching call site : HabUtils::HabUtils.ModuleSimpleEditorTransparency.OnStart
  HabUtils                                1.0.0.0                  1.0.0.0                                           a2f59618ff1be2168281c4f42b1ab4228a8eebfd83f3500489d7fab8e6728ca0

I might want to take a look at the AssemblyLoader code to see what it's actually doing here, but this looks promising!

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Nov 22, 2023

from a side conversation with Dawn. Is this still accurate?

"depends" : [
            { "name" : "NearFutureProps" },
            { "name" : "ModuleManager" }
        ],
        "recommends" : [
            { "name" : "FreeIva" }
        ],
        "suggests" : [
            { "name" : "ShuttleOrbiterConstructionKit" },
            { "name" : "ConnectedLivingSpace" },
            { "name" : "StationPartsExpansionRedux" }
        ]

@dawn0303
Copy link
Contributor

doesnt need the StationPartsExpansionRedux suggestion, if they work fine together and apart then I would probably leave it out

@dawn0303
Copy link
Contributor

for the next release i will probably move the DLL into the GRAPEFRUIT folder if it doesn't conflict

@HebaruSan
Copy link
Member

HebaruSan commented Nov 22, 2023

I do think ksp 1.12.2 or so introduced some protections against multiple DLL installs. So that might be an option if you limit compatibility with those versions.

I believe the sequence was:

  1. Originally it was OK to have duplicate DLLs, and the latest would be used
  2. In KSP 1.12.0, a bug was introduced that caused the game to fail to load if there were duplicate DLLs
  3. The bug was fixed in KSP 1.12.3

linuxgurugamer/QuickMods#18

So if somebody tries this on KSP 1.12.0–2, they might have issues, but they'll probably be having issues anyway.

@JonnyOThan
Copy link
Contributor

So I wonder if we want to be proactive and still create a standalone HabUtils package...certainly don't have to do it now but it will benefit anyone who wants to do the same thing that GRAPEFRUIT is doing.

@dawn0303
Copy link
Contributor

definitely think it would be a good idea

@HebaruSan
Copy link
Member

I think I'd prefer to get a sense of whether Nertea is willing to get involved before we act on that. If he's willing to play along, there's the potential to have a general purpose <18 KB download maintained by the original author.

@dawn0303
Copy link
Contributor

going to update the release to have the DLL bundled in the main folder

@dawn0303
Copy link
Contributor

all updated and bundled together!

Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

This looks pretty solid to me now. @JonnyOThan, I'll leave the merge to you in case you have any loose ends you'd like to tie up.

@dawn0303, after seeing how complicated and brittle some of this packaging/installation stuff can be, I hope it won't come as a surprise when I ask you to please let us know if you make any major changes to this mod, such as switching to a different plugin. We may have to evaluate options again to ensure things keep working. Thanks for the submission!

@dawn0303
Copy link
Contributor

definitely will do! changing modules is going to be when this mod goes into 1.0 so will be revamping everything

@JonnyOThan JonnyOThan merged commit 93da9cb into master Nov 22, 2023
1 check passed
@JonnyOThan JonnyOThan deleted the add/GRAPEFRUIT branch November 22, 2023 20:32
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.

4 participants