search

jsorrell / CarpetSkyAdditions

Empty world generation with new ways to obtain resources
MIT License
455 stars 93 forks source link

Fix the extended death time of coral fans caused partly by the fix of #61 #75

Closed VelizarBG closed 1 year ago

VelizarBG commented 1 year ago

Hi, I've encountered the following problem - when a waterlogged alive coral fan's water gets sucked in by a dispenser, the redstone updates always propagate to the coral first, causing a block update because of these code snippets: https://github.com/jsorrell/CarpetSkyAdditions/blob/e9df9dca13135b23714ee6822b99721cd250b1ec/src/main/java/com/jsorrell/carpetskyadditions/mixin/DeadCoralMixin.java#L31-L37 https://github.com/jsorrell/CarpetSkyAdditions/blob/e9df9dca13135b23714ee6822b99721cd250b1ec/src/main/java/com/jsorrell/carpetskyadditions/mixin/DeadCoralWallFanBlockMixin.java#L22-L27 In this case, that scheduled block event blocks the one which would happen after the water is sucked into the dispenser and which all the other alive coral variants are supposed to get instead. The fix I've offered is to just not let alive coral fan blocks schedule the extra block event which is intended for the dead variants.

jsorrell commented 1 year ago

I merged this, but then thought aren't the checks you added always evaluated as true because the mixin only applies to the dead versions?

jsorrell commented 1 year ago

Yeah, my IDE is saying they always evaluate to true.

VelizarBG commented 1 year ago

@jsorrell Since the alive coral fan versions inherit/extend from the dead versions the check is also being done for them, hence the entire issue and the fix. My changes circumvent exactly that. As to why the IDE gives the warning, my assumption is that it is not aware of the actual inheritance structure because of the mixin context.

jsorrell commented 1 year ago

That's true. Didn't realize the alive inherited from the dead. Yeah, the IDE is not great with mixins for obvious reasons. However, the children (CoralFan, CoralBlock, CoralWallFanBlock) have their own getStateForNeighborUpdate functions which call super, but first call checkLivingConditions which may schedule a tick. Gonna do some testing on how block ticks work.

VelizarBG commented 1 year ago

That's correct, they try to schedule a tick but in the case of a dispenser sucking in their water(activated via redstone line or observer), that exact attempt to schedule a tick ultimately fails because the one added from the mod, intended to only work for the dead coral fan variants and works on the alive ones too, gets scheduled before the one with the normal death time.

jsorrell commented 1 year ago

Yeah, I see. There are other ways to trigger this bug too it seems. Thanks for identifying and fixing this bug. The fix is merged.