Rob23

To fix this, you probably need to remove the quotes around "true" and "false"

This is probably quasi-connectivity (MC-108) and therefore intentional behavior

I have attached a datapack to demonstrate this. To test this bug,

1. Kill an entity in one hit (e.g. fish with axe)

2. Try to revoke the advancement:

   /advancement revoke @s only test:hurt_by_player

You will see the message "Revoked the advancement ..." which is incorrect because the entity was killed a player. Instead the message "Couldn't revoke advancement ... as they don't have it", indicating that the advancement was not triggered.

However, one might argue that this is not a bug. "killed_by_player" is meant as a condition for loot tables and not for advancements. In fact, not even the "player_killed_entity" criterion sets the "killed_by_player" condition.

That being said, I included a version of the advancement that checks for health being 0 instead of using "killed_by_player". This version can be found under test:hurt_by_player2 and does not trigger on kill like expected.

Just did a quick code analysis and it seems like this bug could be solved like this:

net.minecraft.world.entity.Entity

public void load(CompoundTag tag) {
    // ...
    if (tag.contains("CustomName", Tag.TAG_STRING)) { // Existing code, line 1933
        String text = tag.getString("CustomName");
        try {
            setCustomName(Component.Serializer.fromJson(text));
        } catch (Exception ex) {
            LOGGER.warn("Failed to parse entity custom name {}", text, ex);
        }
    } else { // Fix
        setCustomName(null);
    }
    // ...
}

A similar fix can also be applied to other places, specifically command blocks, containers, beacons, banners and enchanting tables.

An alternative way would be to make a method dedicated to retrieving a custom name, with the same code as above, that returns the custom name.

The fix comes from removing the custom name if the tag is not present. Through testing, this seems to work as expected, /data remove does remove the custom name, other types of changes also don't cause any issues.

Yes, exactly. #guarded_by_piglins is only responsible for aggroing piglins when destroying blocks.

Problems from older versions will not be fixed, only the latest versions get updated.

Despite what one might think, #guarded_by_piglins has nothing to do with using/opening the block. Piglins will always be angered by opening chests/barrels/..., no matter whether chests/barrels/... are considered #guarded_by_piglins or not. Therefore this is really the question of whether opening vaults should generally aggro piglins.

For Slow Falling and Levitation:

net.minecraft.world.entity.LivingEntity

public void travel(Vec3 vec) {
    if (isControlledByLocalInstance()) {
        double gravity = getGravity();
        boolean falling = getDeltaMovement().y <= 0;
        if (falling && hasEffect(MobEffects.SLOW_FALLING)) {
            gravity = Math.min(gravity, 0.01);
        }
        // ...
        if (hasEffect(MobEffects.LEVITATION)) {
            motionY += (0.05 * (getEffect(MobEffects.LEVITATION).getAmplifier() + 1) - motion.y) * 0.2;
        }
        // ...
    }
}

Slow falling is just a gravity override (gravity = 0.01) if the gravity was greater.

Levitation gradually increases the motion in the y direction, similarly to falling, however levitation causes 0.8 additional friction, causing the maximum velocity to be reached more quickly. Otherwise it can be modeled by gravity that is approximately -0.000926 times the levitation level, which reaches the same terminal velocity, but more slowly.

For swing duration:

net.minecraft.world.entity.LivingEntity

public static final int SWING_DURATION = 6;
// ...
private int getCurrentSwingDuration() {
    if (MobEffectUtil.hasDigSpeed(this)) {
        return SWING_DURATION - (1 + MobEffectUtil.getDigSpeedAmplification(this));
    }
    if (hasEffect(MobEffects.DIG_SLOWDOWN)
        return SWING_DURATION + (1 + getEffect(MobEffects.DIG_SLOWDOWN).getAmplifier()) * 2;
    }
    return SWING_DURATION;
}

This is currently not controlled by an attribute, it could theoretically be an attribute, but I don't think it would really be that important (could be generic.swing_duration with a base value of 6 and additive modifiers for haste and mining fatigue).

My statement from MC-268417 partially applies here: If Minecraft would use Bogosort, then running /say @e[c=100] should take longer than the heat death of the universe. However, as one can observe, it does not take that long and is done almost instantly.

Code analysis for jump boost

net.minecraft.world.entity.LivingEntity

protected float getJumpPower(float multiplier) {
    return (float) getAttributeValue(Attributes.JUMP_STRENGTH) * multiplier * getBlockJumpFactor() + getJumpBoostPower();
}

public float getJumpBoostPower() {
    return hasEffect(MobEffects.JUMP) ? 0.1f * (getEffect(MobEffects.JUMP).getAmplifier() + 1) : 0;
}

The jump strength is here calculated as follows:

1. Take the generic.jump_strength attribute value

2. Multiply it by the multiplier of the jump (for horses and rabbits)

3. Multiply it by the multiplier of the block you're standing on (0.5 for honey block, 1 for everything else)

4. If you have jump boost, add 0.1 * jump boost level

The problem is that the jump boost effect happens last and after the multiplication of two values that are dependent on the action and the environment.

A good way to observe this is to jump with a horse that has a generic.jump_strength of 0 and jump boost 10. This will cause the horse to always jump the same amount, whether you try to do a small jump or a high jump.
If this behavior should stay, it is pretty much impossible to put the jump boost into the attribute. If, however, the jump boost effect would apply within the attribute and the multipliers all come after, the behavior from before could not be recreated that easily and the jump strength will depend more on these other multipliers.

This is intentional behavior. In order to prevent this, you can modify the generic.fall_damage_multiplier and generic.safe_fall_distance attributes.

Minecraft does not use the Bogosort algorithm, as seen here:

net.minecraft.commands.arguments.selector.EntitySelectorParser

public static final BiConsumer<Vec3, List<? extends Entity>> ORDER_NEAREST = (pos, entities) -> entities.sort((a, b) -> Doubles.compare(a.distanceToSqr(pos), b.distanceToSqr(pos)));
public static final BiConsumer<Vec3, List<? extends Entity>> ORDER_FURTHEST = (pos, entities) -> entities.sort((a, b) -> Doubles.compare(b.distanceToSqr(pos), a.distanceToSqr(pos)));
public static final BiConsumer<Vec3, List<? extends Entity>> ORDER_RANDOM = (pos, entities) -> Collections.shuffle(entities);

Minecraft does in fact use the default Java sorting algorithm provided by List.sort(...).

Quote directly taken from the documentation of List.sort:
"This implementation is a stable, adaptive, iterative mergesort that requires far fewer than n lg(n) comparisons when the input array is partially sorted, while offering the performance of a traditional mergesort when the input array is randomly ordered."
https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/List.html#sort(java.util.Comparator)

The simplest way to confirm that Minecraft does in fact not use Bogosort is to try to sort bigger amounts of entities. If Minecraft would use the Bogosort algorithm, sorting 100 entities should take approximately 10^151^ longer than sorting 10 entities. However, 100 entities can in fact be sorted by Minecraft in very few time.

The only concern is that, when specifying a selector @e[limit=...,sort=nearest], Minecraft will sort all of the entities before applying the limit. This causes the sorting process to be much slower than necessary. For example an @e[limit=1,sort=nearest] selector will run in O(n log(n)) where n is the total amount of entities, where it could run in O(n) time, going through every entity once. This could be solved by using bubblesort if the limit is low in comparison to the amount of entities. The interesting fact about bubblesort is, that after k iterations, the last k elements will be correctly sorted. Therefore, you could apply k=limit iterations of bubblesort on the n entities, resulting in a time complexity of O(nk). This can be beneficial if the amount of entities is in the thousands and we only want 2 or 3 entities.

Conclusion: Minecraft doesn't use Bogosort, however the sorting process might be accelerated by using bubblesort if the amount of entities sorted is much higher than the wanted limit (e.g. 1000 compared to 3).

Code analysis for block breaking

net.minecraft.world.entity.player.Player

public float getDestroySpeed(BlockState block) {
    float speed = inventory.getDestroySpeed(block);
    if (speed > 1) {
        int effLevel = EnchantmentHelper.getBlockEfficiency(this);
        ItemStack item = getMainHandItem();
        // Efficiency
        if (effLevel > 0 && !item.isEmpty()) {
            speed += effLevel * effLevel + 1;
        }
    }
    // Haste
    if (MobEffectUtil.hasDigSpeed(this)) {
        speed *= 1 + (MobEffectUtil.getDigSpeedAmplification(this) + 1) * 0.2f;
    }

    if (hasEffect(MobEffects.DIG_SLOWDOWN)) {
        float multiplier;

        // Mining Fatigue
        switch (getEffect(MobEffects.DIG_SLOWDOWN).getAmplifier()) {
        case 0:
            multiplier = 0.3f;
            break;
        case 1:
            multiplier = 0.09f;
            break;
        case 2:
            multiplier = 0.0027f;
            break;
        case 3:
        default:
            multiplier = 0.00081f;
            break;
        }
        speed *= multiplier;
    }
    // Attribute multiplier
    speed *= (float) getAttributeValue(Attributes.BLOCK_BREAK_SPEED);

    if (isEyeInFluid(FluidTags.WATER) && !EnchantmentHelper.hasAquaAffinity(this)) {
        speed /= 5f;
    }
    if (!onGround()) {
        speed /= 5f;
    }

    return speed;
}

In this code, the following operations are applied to the block breaking speed:

1. If you have efficiency, add efficiency level²+1

2. Multiply by haste level * 0.2 + 1

3. If you have mining fatigue, multiply by 0.3 for level 1, 0.09 for level 2, 0.027 for level 3 and 0.0081 for level 4 and beyond

4. Multiply by attribute value

5. If in water, divide by 5

6. If not on ground, divide by 5

Step 1 can't be put into the attribute because it depends on the block you are breaking.

Steps 2 and 3 can be put into attribute modification due to being multiplication.
Steps 5 and 6 might be put into attribute modification, however they depend on the environment.

Note: Step 3 might be slightly difficult, since attribute modification is usually linear and here, exponential

Duplicate of MC-268362

Affects 24w03a

This is intentional behavior: Any items that would usually stay in a crafting table are ejected to make sure the crafter is empty after crafting and ready to craft again. Leaving them inside would not make any sense because then the spaces are occupied and therefore in the way for the next crafting recipe.

Did a code analysis, it seems like Minecraft just discards all attribute modifiers where the first two or the last two numbers of the UUID are 0:

net.minecraft.world.item.ItemStack

public Multimap<Attribute, AttributeModifier> getAttributeModifiers(EquipmentSlot slot) {
    // ...
    AttributeModifier modifier = AttributeModifier.load(tag);
    if (modifier == null) {
        continue;
    }

    if (modifier.getId().getLeastSignificantBits() != 0L && modifier.getId().getMostSignificantBits() != 0L) {
        map.put(attribute, modifier);
    }
    // ...
}

If anything, this is a feature request. Currently, to work around this, you can use function macros in data packs.

The issue is not completely gone, the success value now seems to be assigned correctly, however, it is still not possible to distinguish between a function that failed and a function that returned 0, the message is still "Function ... returned 0", which is misleading.