PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

43M Downloads

Better CofH Core / Holding Enchant Integration

HipHopHuman opened this issue ยท 5 comments

commented

PNC supports the Holding enchantment added by CofH Core, but this enchantment isn't registered by CoFH Core. It's registered by Thermal Foundation. All the code for it exists in CofH Core, but the function exposed by Core (cofh.core.init.CoreEnchantments.registerHoldingEnchantment()) isn't called by Core - it's called by Thermal Foundation.

I've been poking around the CofH Discord trying all manner of ways to get this working without installing Thermal Foundation, but short of creating a bridge mod myself just for this purpose, the next easiest option is to install the entirety of Thermal Foundation, disable everything in it's config and then devise a plethora of custom KubeJS scripts that disables everything else, like the recipes and JEI info.

I keep re-iterating the fact that I'm not a mod dev and a lot of this jargon flies right over my head, but:

It was suggested to me on the CoFH Discord that it would make more sense and be a lot cleaner if this were handled internally in PNC - either by making CofH Core an optional dependency (making sure to be aware of classloading), checking if CofH core is loaded in the modlist and calling registerHoldingEnchantment() in a PNC Class that is only accessed after the presence check while also making sure to check if Thermal Foundation is present so that registerHoldingEnchantment() isn't accidentally called twice.

Another thing that was suggested (as a worst case scenario) was using Java reflection (which I know zilch about).

In any case, I think giving packmakers the option to enable the Holding enchantment sans-Thermal would be a nice feature.

commented

Calling registerHoldingEnchantment() from the integration would be easy enough, I think. It's easy to check if Thermal is present or not.

commented

A potential concern just occurred to me for this suggestion regarding a very edge-case thing - but what if another mod that is not Thermal does the same thing? It would check if thermal is present, if not, call .registerHoldingEnchantment() - and then if PNC does the same, you still have a double registration ocurring. It may not be a concern at all for now, but for the sake of future-proofing - is there a better way? Can one instead check if registerHoldingEnchantment() was already called, instead of checking if a mod that does so exists?

commented

Looking at the CoFH code, it appears that CoreEnchantments.registerHoldingEnchantment() is quite safe to call multiple times. If the enchantment is already registered, the call is effectively a no-op.

commented

That's a relief!

commented

Added in 2.15.1