[1.18-pre8] Code review/input

[kwpugh]

Hi, I'm working on an enchantment like the classic Soul Bound that allows the player to "keep" the enchanted item after death.

Full GitHub: https://github.com/kwpugh/MoreGems/tree/1.18-pre8

I approached it by mixing into 2 classes.

Mixin#1:
@mixin(ServerPlayerEntity.class)
public abstract class ServerPlayerEntityMixinBound
{
@Inject(method="copyFrom", at = @at(value="RETURN"))
public void moregemsCopyFrom(ServerPlayerEntity oldPlayer, boolean alive, CallbackInfo ci)
{
if(!alive)
{
ServerPlayerEntity currentPlayer = (ServerPlayerEntity) (Object) this;
boolean ruleTest = currentPlayer.world.getGameRules().getBoolean(GameRules.KEEP_INVENTORY);

        if(!ruleTest && !oldPlayer.isSpectator())
        {
            currentPlayer.getInventory().clone(oldPlayer.getInventory());
        }
    }
}

}

This seemed necessary to effectively copy the player inventory from old to new player.

Mixin#2
@Inject(method = "dropItem(Lnet/minecraft/item/ItemStack;ZZ)Lnet/minecraft/entity/ItemEntity;", at = @at("HEAD"), cancellable = true)
private void moregemsDropItem(@NotNull ItemStack stack, boolean throwRandomly, boolean retainOwnership, CallbackInfoReturnable cir)
{
PlayerEntity currentPlayer = (PlayerEntity) (Object) this;
World world = currentPlayer.world;

    if (!stack.isEmpty())
    {
        if(EnchantmentHelper.getLevel(EnchantmentInit.BOUND, stack) > 0)
        {
            if(!world.isClient)
            {
                currentPlayer.giveItemStack(stack);
            }

            if(world.isClient)
            {
                cir.cancel();
            }
        }
    }
}

With this mixin, I wanted to perform the check and return null, but that would work. I resorted to giving the player back the stack if it has the enchantment on it.

Questions:

1. Is this a safe approach?
2. Is there a better/safer approach?

This works, but there is a delay after the player respawns, before the items reappear in inventory. Works on both SP and MP. It would be nice to have the items to remain in the same slots they were in prior to death.

Thank you for any input I receive.

[warjort]

I can't really comment on the overall approach (I don't know that much about this area of the game).

But your approach does seem reasonable.

• Stop the player from dropping certain items
• Add a copy inventory to retain those items if minecraft doesn't copy them because of game rules

You know fabric already has events to handle player deaths
https://github.com/FabricMC/fabric/blob/1.18/fabric-entity-events-v1/src/main/java/net/fabricmc/fabric/api/entity/event/v1/ServerPlayerEvents.java

For retaining inventory position, I think you need to mixin the PlayerInventory.dropAll() and either

• stop the dropItem/clear from getting called at all for your items
• after the drop happens, remove the new item entity and put the stack back in position (i.e. effectively pick it back up)

[kwpugh]

Oh, well, I didn't know that. That would be a lot easier. Thanks for telling me about that.