Origins (Forge)

Origins (Forge)

7M Downloads

Bug (+ a fix i made): most Block_Conditions do not work with the ReplaceLootTable power type

Marco6789 opened this issue ยท 2 comments

commented

There exists a bug that causes most block condition types to not work when replacing a loot table via a datapack. This bug is caused by the 'doesApply()' method of the 'ReplaceLootTableConfiguration' class. The latest if-statement checks the 'LootContextParams.ORIGIN' of the block, and then checks the position in the world to get the blockstate. However, at the time this position is checked, the block at that position is already broken and has changed into a different (presumably null or minecraft:air) block. The fix for this is simple: in stead of checking the 'LootContextParams.ORIGIN' in the if-statement, check the 'LootContextParams.BLOCK_ENTITY'. I have already implemented this in a fork of the project i made to test it, and it is working as expected now.

The full filepath to the bugged lines is:
apoli/src/main/java/io/github/edwinmindcraft/apoli/common/power/configuration/ReplaceLootTableConfiguration.java.

The lines causing the bug are located in the doesApply method, lines 93-96:
if(lootContext.hasParam(LootContextParams.ORIGIN)) {
BlockPos blockPos = new BlockPos(lootContext.getParam(LootContextParams.ORIGIN));
return ConfiguredBlockCondition.check(blockCondition(), lootContext.getLevel(), blockPos);
}

The fix is to replace these lines with the following:
if(lootContext.hasParam(LootContextParams.BLOCK_ENTITY)){
BlockEntity blockentity = lootContext.getParam(LootContextParams.BLOCK_ENTITY);
return ConfiguredBlockCondition.check(blockCondition(), lootContext.getLevel(), blockentity.getBlockPos(), () -> blockentity.getBlockState());
}

a new import statement is also needed to get the BlockEntity type:
import net.minecraft.world.level.block.entity.BlockEntity;

I found this bug in the 1.19.x/1.6 branch of the apoli project, but it is presumably present in later branches that have this power as well.
I hope this helps!

commented

As it turns out, the coremod that makes replace loot table work wasn't applied correctly. I have since rewritten how the power type works, to not require coremods.

commented

I believe LootContextParams.BLOCK_STATE should be used instead. If LootContextParams.BLOCK_ENTITY were to be used, then blocks that do not have a block entity wouldn't be evaluated since the block entity parameter in a block loot context is an optional parameter (e.g: nullable) while the block state parameter in a block loot context is a required parameter