search

UmbraSpaceIndustries / USITools

Shared components for KSP mods
Other
39 stars 38 forks source link

Add backward-compatibility patch for converters relevant to life support #134

Closed Wyzard256 closed 5 years ago

Wyzard256 commented 5 years ago

This patch attempts to upgrade parts that rely on certain obsolete USI PartModules, to use the new USI_Converter system instead. It focuses on modules that are directly needed for life support (habs, recyclers, greenhouses), and doesn't attempt to handle things like resource harvesting. Though it works OK for many parts, there's no guarantee that it correctly handles all of them, and it is not intended as a long-term solution; maintainers of mods that integrate with USI should still update their own integration patches. This patch is just meant to get most parts into at least a usable state until then.

Wyzard256 commented 5 years ago

@DoktorKrogg may want to look at this.

Wyzard256 commented 4 years ago

This was merged recently, but was then deleted by commit e677a4197301b455a1eb363d590cdbcf3d294578 ("1.7.x Compat Patch"), so it didn't end up in the release. That commit didn't update the part configs to not need the compatibility patch, so I'm guessing the deletion was unintentional. Should I submit another merge request to add it back?

tjdeckard commented 4 years ago

I appreciate the work that went into making this patch and I don't mind having it available to folks who want to use it. That said, I still resist the idea of incorporating into the official release a patch that is meant to be temporary. Without a clear definition of what "temporary" means and without someone to enforce it, the patch will end up becoming permanent. We're quickly coming up on a year already since the converters change were made and that's about as long as I would want to have a stop-gap like this in place anyway.

Wyzard256 commented 4 years ago

Seems I didn't check carefully enough: commit e677a4197301b455a1eb363d590cdbcf3d294578 didn't update any parts to use the new modules, because USI Tools doesn't even contain any parts, but it looks like things have been updated in other commits in other projects. The only parts in my install that are still relying on the patch are the GroundConstruction workshops.

That said, it wouldn't hurt to treat the patch itself as permanent; the "temporary" bit refers to its fixups for any particular part, which it'll stop doing once that part has been updated to stop using the old modules. Over time, the number of mods that rely on the patch will decrease, but until that number reaches zero, it's still providing value, and there's no real harm in having it.

(Also, the patch helps to identify parts that ought to be updated, since it adds a MODULE block named "ThisPartUsesObsoleteUsiModules" to each patched part, specifically to produce warnings in the game log. Search for that string in your ModuleManager.ConfigCache and you'll find parts that rely on the patch. I included that to help raise awareness of the issue and encourage people to update their parts.)

tjdeckard commented 4 years ago

I disagree but if it has already made it into the release then it's a moot point.