Mekanism

Mekanism

111M Downloads

Move capabilities package to the API?

tomdodd4598 opened this issue · 9 comments

commented

Describe the the feature you'd like
Currently NuclearCraft has to use some slow reflection methods to get at the gas and tube connection capabilities for its machines' gas support. Would it be an idea to move the capabilities package to the API section of the mod to allow NC and perhaps other mods to get at these directly?

Describe alternatives you've considered
The reflection methods are, as with any reflection, rather slow, so in general this causes the machines to take up more tick time.

commented

I think it should be possible to do something, not quite sure of the best way of going about it, will have to discuss it with @thiakil . In part because I know some addons at the very least Gas conduits (which I can always just release an update for) instead of just building against the API build against the mekanism jar so can just reference the capabilities directly. Though I think one solution which would maintain backwards compat might be to add getters in the API so that it is possible to get the references from the API (while maintaining backwards compat). On a side note, as of Mekanism 9.7.0 the TUBE_CONNECTION_CAPABILITY actually does not do anything, as I went through cleaning things up so that it all works properly with the GAS_HANDLER_CAPABILITY instead of having to implement/look at two different capabilities. (Though at least for now I suppose it doesn't hurt to leave support in NC for it so that you are compatible with older versions of mekanism as well.)

I will try to figure something out for the next release that allows accessing the capability references via the API (though I may leave out the TUBE_CONNECTION_CAPABILITY as it is deprecated, doesn't do anything, and if a mod requires a version of mekanism that has a way to get the capabilities from API anyways, it will be a version that the tube cap already does not do anything.

commented

Fantastic - should have thought to build against the jar, but thanks for the heads up :)

commented

Actually slight question, how much stuff would you need moved/access to? I am assuming just the caps in this class? https://github.com/mekanism/Mekanism/blob/1.12/src/main/java/mekanism/common/capabilities/Capabilities.java

Or are there other things you would also like access to about the caps. (Rather than me going through NC and seeing what things are used)

commented

I suppose just the capabilities in that class, yes, though from a 'code aesthetics' point of view I wasn't sure how nice of a change splitting up the package would be.

commented

Ok, well I will think about it and discuss with thiakil what the best way of doing it is. I also probably will ping you on the PR (given we tend to make bigger changes through PR for reviewing purposes) once we make it so you can put your input on the changes if they are reasonable or if there are any other changes/usability things about it that would be nice.

commented

I don't see anything in that package that needs splitting out? It only has the junk default implementations and the registration in the linked file.

You should have your own @CapabilityInject annotated fields in your own classes, there's no need to use ours. Forge will inject the same value to both.

commented

Ah, forgot about that - all fields annotated with @CapabilityInject reference the same capability instance, right? So I could just have this written somewhere and use that in the logic?

commented

Yep, that's the intended use - as long as the interface name matches they'll be set to the same thing. You just need to null check it (or check that Mekanism is loaded of course)

commented

Aha, okay, that should defo to the job. No worries then, don't move anything! Thanks for the help :)