Add BlockItemProvider capability to allow alternative sources of blocks for modifiers like Exchanging #5568
Conversation
|
|
||
| /** Event listener to attach the capability */ | ||
| private static void attachCapability(AttachCapabilitiesEvent<ItemStack> event) { | ||
| if (event.getObject().getItem() instanceof BlockItem block) { |
There was a problem hiding this comment.
Attaching this to every block item feels a bit wasteful, especially with how 1.20 capabilities are implemented.
Why not just have a fallback if the stack lacks the capabilty?
There was a problem hiding this comment.
Its a lot cleaner, as the alternative of special casing for everything that uses this capability (probably only two things) is quite ugly imo (and can lead to issues such as not being able to override the block a BlockItem provides if a consumer of the capability mistakenly orders the checks the wrong way around).
It also acts as an example to other implementers, though thats a very weak reason.
There was a problem hiding this comment.
In your experience, how notable is the cost of such attachments? I know firing the event is expensive, but it might be once you need to fetch caps anyways that cost is unavoiable.
There was a problem hiding this comment.
I haven't done any specific profiling, but I rarely see cap stuff come up in spark profiles and when it does its usually Curios capability providers which do expensive stuff for some reason (can't remember the specifics).
In this particular case its just allocating a single object, which should be fine especially given the overhead of the event anyway. Some caching could be implemented but I doubt that is worth it.
This could always be done by putting the capability in charge of placing the block to some degree. Default impl runs block item placement, but you could do an impl for placement from other sources. Related, if we are going to place in exchanging here, we should place from ichors fluid effect in the same way. We do not, however, need to use this capability for the fixed block forms of fluid effects, just the one that selects block from the offhand. |
KnightMiner
added
Technical
After implementing this I have realised that theres nowhere in Tinkers (or in the compat I had planned for Botania) where this would actually be used, as they all consume from the inventory anyway. This means I can't test it without some annoying additional useless code. I've committed and pushed this code to my fork on a different branch in case its useful to anyone, but will leave it out of this PR. |
2 participants
This adds a capability
BlockItemProviderCapabilitythat can be added to ItemStacks to let them provide block items. This is intended for compatibility with other mods like Botania that add items that place blocks at the cost of things such as Mana, or hold large numbers of a single block.BlockItem rather than Block was chosen as the current exchanging logic works off BlockItem, however looking into other places that use
instanceof BlockItemthere are alternatives for placing the block 'manually' which may be added to Exchanging and the capability swapped to directly use Blocks.TODO: