Engineer's Decor

Engineer's Decor

19M Downloads

Conflict with PMMO causing Labeled crates to void items

HopsandBarley opened this issue · 12 comments

commented

There is a conflict between Engineer's decor labeled crates and Project MMO, that is causing contents of Labeled crates to be voided when the crates are moved or broken. Do no know the exact conflict, but when PMMO is uninstalled, the bug goes away. (1.16)

Likely an issue with PMMO, but I dont know for sure, so I wanted to bring it to your attention.

commented

Same issue on Enginners Life 2.

commented

Hi guys, I gonna double check on the ED side, too. Is there already related issue dropped at pmmo so that @Harmonised7 knows? Cheers!

commented

Hi, yes it's post in bug section on Discord's PMMO. Have a good day.

commented

I can confirm that LootContext code of Project MMO is causing this, looking into it now. If someone finds something out, please let me know on Discord. (I will likely not be replying anywhere other than Discord)

Edit: drops = block.getDrops( event.getState(), builder ); is the method that destroys the items. I do not think that this should happen, so I am assuming it's not my side that's causing this. Keep in mind that I don't really understand LootContext related things, though I give all the arguments to my builder that the event provides, this includes: Player Position, Tool used, Player Entity, and Tile Entity

commented
commented

Hi Harmonised, thanks for the quick check an keeping me in the loop. Let me take a look in the code of the Crate if I can find an problem there, too. Don't worry about the emoji, it's very likely mis-clicked, people are normally happy when modders care and maintain - and PMMO is a really nice mod. Cheers!

commented

Hi, thanks.Harmonised.

commented

Ok, quickly double checked. So the getDrops(...) of the crate depends on the loot context explosion radius (not null and >0) and that the tile entity is still available when getDrops(...) is called. Then it gathers the TE contents into item nbt and returns the crate item. The call is indirect via a super class (because I have more blocks keeping their inventory). Could we have a collision there?

In code, this would be these methods, getDrops() is here:

@Override
@SuppressWarnings("deprecation")
public List<ItemStack> getDrops(BlockState state, LootContext.Builder builder)
{
final ServerWorld world = builder.getWorld();
final Float explosion_radius = builder.get(LootParameters.EXPLOSION_RADIUS);
final TileEntity te = builder.get(LootParameters.BLOCK_ENTITY);
if((!hasDynamicDropList()) || (world==null)) return super.getDrops(state, builder);
boolean is_explosion = (explosion_radius!=null) && (explosion_radius > 0);
return dropList(state, world, te, is_explosion);
}

... and the Crate drops here:

@Override
public List<ItemStack> dropList(BlockState state, World world, final TileEntity te, boolean explosion)
{
final List<ItemStack> stacks = new ArrayList<ItemStack>();
if(world.isRemote) return stacks;
if(!(te instanceof LabeledCrateTileEntity)) return stacks;
if(!explosion) {
ItemStack stack = new ItemStack(this, 1);
CompoundNBT te_nbt = ((LabeledCrateTileEntity)te).reset_getnbt();
CompoundNBT nbt = new CompoundNBT();
if(!te_nbt.isEmpty()) nbt.put("tedata", te_nbt);
if(!nbt.isEmpty()) stack.setTag(nbt);
Auxiliaries.setItemLabel(stack, ((LabeledCrateTileEntity)te).getCustomName());
stacks.add(stack);
} else {
for(ItemStack stack: ((LabeledCrateTileEntity)te).main_inventory_) stacks.add(stack);
((LabeledCrateTileEntity)te).reset_getnbt();
}
return stacks;
}

You can also paste the permalink of the caller side in PMMO if you like an additional pair of eyes checking. Cheers!

commented

I've found an inefficient solution to this issue, which will be fixed in 3.52.5

P.S. Whoever left the Thumbs Down emoji, that's super inconsiderate... Your actions affect people, and in this case, rather negatively...

commented

Hi again. Alright, I'd say your code looks pretty much ok. One thing I noticed in the ED mod that can cause the problem, too:
Querying the Crate drops will clear its inventory after gathering the TE data. That is a workaround for a drop dupe issue in the 1.14ish MC version range - which I did not take out yet. At least until now. So, if another event callback queries the Crate drops, but does not actually drop them, then your callback (probably last due to low prio) will see an empty crate. "Hence", there will be also a fix from the ED side ;). Cheers!

commented

Okay, the update is on curse, closing then, cheers guys!