Fabric API

Fabric API

106M Downloads

DynamicAttributeTool#getDynamicModifiers is bugged

Draylar opened this issue ยท 0 comments

commented

It has come to my attention that the getDynamicModifiers method in DynamicAttributeTool is bugged. This is due to a UUID not being specified in AttributeManager#mergeAttributes. A random UUID is chosen when one is not specified, so the game fails to remove the attribute when needed. This results in the attribute being stacked[?] every time you select a DynamicAttributeTool in your hotbar.


Context / Problem:

Item has a method called getAttributeModifiers. This method returns the modifiers that should be added to the player/entity when the item is equipped and is called in LivingEntity#method_30129. Fabric provides DynamicAttributeTool, which has a getDynamicModifiers method. This method, unlike getAttributeModifiers, gives you context on ItemStack and the entity holding the item.

When the player switches an item around, the attribute modifiers for the previous stack and the new stack are calculated. The old modifiers are removed, and the new modifiers are added. An attribute is properly removed if the UUID of the attribute that is being removed matches an existing UUID (Map<UUID, Modifier>). The important thing to note is that if the modifiers are the same, but the UUIDs are different, the old modifier will not be removed.

When getDynamicModifiers is called, the results are merged with the result of getAttributeModifiers. This is done to avoid having multiple attributes of the same type in the tooltip (such as +4 Damage and +2 Damage on 2 separate lines). The problem is that the entity attribute modifiers created in the merge method do not specify a UUID, which means the method will not return the same results when called multiple times in a row. When the "remove old modifiers," logic runs, it fails, because the UUIDs are different.


Reproducing:

public class TestItem extends Item implements DynamicAttributeTool {

    public static final UUID TEST_UUID = UUID.fromString("CB3F55D3-645C-4F38-A497-9C13A33DB5CF");

    public TestItem(Settings settings) {
        super(settings);
    }

    @Override
    public Multimap<EntityAttribute, EntityAttributeModifier> getDynamicModifiers(EquipmentSlot slot, ItemStack stack, @Nullable LivingEntity user) {
        if(slot.equals(EquipmentSlot.MAINHAND)) {
            ImmutableMultimap.Builder<EntityAttribute, EntityAttributeModifier> builder = ImmutableMultimap.builder();
            builder.put(EntityAttributes.GENERIC_MOVEMENT_SPEED, new EntityAttributeModifier(TEST_UUID, "TEST", -1.0, EntityAttributeModifier.Operation.ADDITION));
            return builder.build();
        } else {
            return EMPTY;
        }
    }
}

Resulting - streamable
Note how the attribute persists despite me switching off the item.


I will be looking into writing a fix for this soon, but I wanted to post an issue first for additional context when that PR arrives.