[1.20.1 Fabric] Setting itemStack to null in EntityShapeContext breaks Bumblezone's brushable pollen block
TelepathicGrunt opened this issue ยท 2 comments
Using lithium-fabric-mc1.20.1-0.11.2.jar and latest Bumblezone, attempting to brush a Suspicious Pile of Pollen block will do nothing.
The problematic mixin:
The issue comes down to me needing to use the EntityShapeContext's itemstack field directly so I can check if the item is in my brushing allowed tag. The isHolding method is no sufficient to my needs. However, Lithium sets this field to null which causes my block to never be brushed because it thinks player held item is null which is wrong.
https://github.com/TelepathicGrunt/Bumblezone/blob/cf5e4c67ba5de7713458651f05d888cc884b4752/common/src/main/java/com/telepathicgrunt/the_bumblezone/blocks/PileOfPollenSuspicious.java#L126
I originally got told about this issue because in past Bumblezone version, this conflict crashed my mod due to Canary copying the same mixin from lithium. I put out a null check fix but realized now that it means while the crash is stopped, the block is no longer brushable. I checked Fabric and Lithium also causes the issue with the mixin
AbdElAziz333/Canary#182
TelepathicGrunt/Bumblezone#299
I have suspicion on if this null field setting is even giving any tangible performance benefit... In general, itemstacks should never be null. As of now, I need to hack around Lithium to get the actual held item from the context again. What I may have to do is call isHolding directly once with a dummy input to force Lithium to init itemstack. Then get the field.
I wanted to grab the item directly from the context in case other mods make their own context with the itemstack set directly like a fake player or some sort of autobrushing machine which is why I can't assume that I can call ((LivingEntity)this.entity).getMainHandStack() myself.
Also, if a mod tries to say automate brushing line-of-sight machine and does so by crafting their own EntityShapeContext to pass into brushable block's various methods, this mixin is assuming the itemstack is always ((LivingEntity)this.entity).getMainHandStack() which it could not be in this case and ends up overwriting the original itemstack that the machine was trying to pass into EntityShapeContext. basically this mixin makes lots of assumptions I am not comfortable with.
