fix(Rune): Rune effect loss when placed soulbound/fireproof blocks are broken#1063
fix(Rune): Rune effect loss when placed soulbound/fireproof blocks are broken#1063TagDL wants to merge 7 commits into
Conversation
JustAHuman-xD
left a comment
There was a problem hiding this comment.
We actually have a system for attaching data directly to blocks, instead of using the chunk pdc like you do here!
If you take a look at some of rebar's built-in interfaces you should be able to find some examples so that you can swap to it. (VirtualInventoryRebarBlock, RecipeProcessorRebarBlock, EntityHolderRebarBlock, to name a few)
Additionally the logic for retaining both runes, soulbound & fireproof should be inside of the corresponding classes in a listener, not inside this abstract class here.
When you move it to both I'd also suggest adding a pdc key to the fireproof rune, and checking for that instead of just the datatag, you could also add some util methods for both which you could use for this. (SoulboundRune#isSoulBound, FireproofRune#isFireproof, for ex)
Overall thank you for the PR! I'm sure with a few changes this can definitely be added
…, add RebarBlockUnloadEvent
|
Tested that this solution fixes rebar blocks, but it does not address vanilla blocks and we definitely should as it's more or less the same issue. Unfortunately I think we would need to switch back to using chunk's PDC to fix this for normal blocks, not using the approach Justin suggested. @JustAHuman-xD do you have any input since you reviewed it originally? |
Discussed in VC - I think using the chunk's PDC is still the best way forward. Could you change it back to that and handle the vanilla case? Sorry for the confusion |
Already change it to chunk's PDC❤️ |
Fixed a bug where RebarBlocks with Soulbound Rune and Fireproof Rune effects would lose both modifications upon being placed and subsequently broken by players.