Fix Contraption voiding Container content (#5420) #8774
Conversation
Allmoz
force-pushed
the
contraption_conteiner
branch
from
fa6b195 to
73635d2
Compare
VoidLeech
added
pr type: fix
| CompoundTag tag = block.nbt(); | ||
| if(tag != null){ | ||
| BlockEntity blockEntity = BlockEntity.loadStatic(targetPos, state, tag); | ||
| if (blockEntity instanceof Container container) |
There was a problem hiding this comment.
Container is probably (definitely, the Placard for example already doesn't implement it) too limited.
There was a problem hiding this comment.
The issue i have found is that most methods access the block entity using the Position and Level directly, so they don't work in this case where the block entity is not actually anywhere. I was thinking to maybe use Item Requirements from the schematicanon implementation, it seems to complement this approach (would not work with placards until #8550). To me seems like a weird way to patch it and i'm not sure is desirable in the codebase, but gonna give it a try if no one stops me.
There was a problem hiding this comment.
Yeah tricky I don’t know what the proper solution would be. Using item requirements wouldn’t work though. Storage contents are almost never included in that (for good reason).
There was a problem hiding this comment.
I was adding exceptions for create containers, but maybe SmartBlockEntity.destroy() is enough for the majority of cases
Allmoz
force-pushed
the
contraption_conteiner
branch
2 times, most recently
from
77e54dc to
845d199
Compare
|
@VoidLeech :) added SmartBlockEntity, and sadly also ItemVaultBlockEntity, since it seems that it does not implement the destroy behavior like the others. dunno though if there are other vanilla or create exceptions that are not being covered, from the top of your head do you know other entity block that might not be covered? edit: placard of course... |
Allmoz
force-pushed
the
contraption_conteiner
branch
from
845d199 to
78a793b
Compare
|
Tested vanilla, the only issue i found is Shulkers Boxes content being dropped (should I add an exception?) Belts...are a whole other problem, content gets correctly dropped, but not shafts, and belts get duplicated (it a new dupe even without this PR). this one i don't know how to approach, i probably would just let the belt of that particular block get lost, is that ok? (it would not fix the dupe, but it would not make it bigger) |
|
The belt case is similar to #5879. the only difference is that is not a piston taking out the belt segment, is the belt is being placed over a block that cannot be mined (tested it with bedrock) |
Allmoz
force-pushed
the
contraption_conteiner
branch
from
78a793b to
4a4a004
Compare
|
small test setup |
Allmoz
force-pushed
the
contraption_conteiner
branch
from
4a4a004 to
f50f4cd
Compare
|
Added all exceptions i could find following the original logic of each block. Had to add two getters, I followed the style guide of the neighbouring code or similar blocks. @VoidLeech 🫡 |
Is this a valid aproach to fix #5420 ?
the method seems like a delicate area, so I keep it minimal