Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Marco6789 opened this issue Dec 27, 2023 · 2 comments

Comments

@Marco6789
Copy link

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!

@eggohito
Copy link

eggohito commented Jan 5, 2024

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

@MerchantPug
Copy link
Collaborator

MerchantPug commented Mar 12, 2024

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.

MerchantPug added a commit to EdwinMindcraft/apoli that referenced this issue Mar 12, 2024
MerchantPug added a commit to EdwinMindcraft/apoli that referenced this issue Mar 13, 2024
MerchantPug added a commit to EdwinMindcraft/apoli that referenced this issue Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants