From 60f3bd8d0c01e17f1947e706b5c3ab5fda4b694a Mon Sep 17 00:00:00 2001 From: Bjarne Koll Date: Sun, 16 Jun 2024 12:44:22 +0200 Subject: [PATCH] De-deprecate BlockData#getDestroySpeed --- patches/api/0214-Add-Destroy-Speed-API.patch | 14 +- patches/api/0357-Block-Ticking-API.patch | 6 +- ...he-collision-shape-of-a-block-before.patch | 2 +- .../server/0436-Add-Destroy-Speed-API.patch | 204 ++++++++++++++++-- patches/server/0736-Block-Ticking-API.patch | 4 +- ...he-collision-shape-of-a-block-before.patch | 2 +- 6 files changed, 205 insertions(+), 27 deletions(-) diff --git a/patches/api/0214-Add-Destroy-Speed-API.patch b/patches/api/0214-Add-Destroy-Speed-API.patch index 509bde4c8a..d6e08a09df 100644 --- a/patches/api/0214-Add-Destroy-Speed-API.patch +++ b/patches/api/0214-Add-Destroy-Speed-API.patch @@ -42,10 +42,10 @@ index 25db31b2e9a6d75f0c59f75237842f9ad7d1c350..75c2aadb0a2baebe8b2625ad11b16380 + // Paper end - destroy speed API } diff --git a/src/main/java/org/bukkit/block/data/BlockData.java b/src/main/java/org/bukkit/block/data/BlockData.java -index cd3b3e05cc825cfedec07f9a2a1e0b7b2a8866d6..a2dc7376b2a3d386b671c894f73389139e0d97bf 100644 +index cd3b3e05cc825cfedec07f9a2a1e0b7b2a8866d6..890a511355dd3f2aa9330fdc72c0fb4b3e44e5cb 100644 --- a/src/main/java/org/bukkit/block/data/BlockData.java +++ b/src/main/java/org/bukkit/block/data/BlockData.java -@@ -266,4 +266,35 @@ public interface BlockData extends Cloneable { +@@ -266,4 +266,33 @@ public interface BlockData extends Cloneable { @NotNull @ApiStatus.Experimental BlockState createBlockState(); @@ -58,10 +58,9 @@ index cd3b3e05cc825cfedec07f9a2a1e0b7b2a8866d6..a2dc7376b2a3d386b671c894f7338913 + * + * @param itemStack {@link ItemStack} used to mine this Block + * @return the speed that this Block will be mined by the given {@link ItemStack} -+ * @deprecated the destroy speed of a block was never purely tied to an item stack. Since 1.21 enchantments -+ * also use complex effects that require a consuming player to compute their effects, including mining efficiency. ++ * @apiNote this method assumes default player state and hence, e.g., does not take into account changed ++ * player attributes or potion effects. + */ -+ @Deprecated(forRemoval = true, since = "1.21") + default float getDestroySpeed(final @NotNull ItemStack itemStack) { + return this.getDestroySpeed(itemStack, false); + } @@ -74,10 +73,9 @@ index cd3b3e05cc825cfedec07f9a2a1e0b7b2a8866d6..a2dc7376b2a3d386b671c894f7338913 + * @param itemStack {@link ItemStack} used to mine this Block + * @param considerEnchants true to look at enchants on the itemstack + * @return the speed that this Block will be mined by the given {@link ItemStack} -+ * @deprecated the destroy speed of a block was never purely tied to an item stack. Since 1.21 enchantments -+ * also use complex effects that require a consuming player to compute their effects, including mining efficiency. ++ * @apiNote this method assumes default player state and hence, e.g., does not take into account changed ++ * player attributes or potion effects. + */ -+ @Deprecated(forRemoval = true, since = "1.21") + float getDestroySpeed(@NotNull ItemStack itemStack, boolean considerEnchants); + // Paper end - destroy speed API } diff --git a/patches/api/0357-Block-Ticking-API.patch b/patches/api/0357-Block-Ticking-API.patch index 5371277e73..1836045273 100644 --- a/patches/api/0357-Block-Ticking-API.patch +++ b/patches/api/0357-Block-Ticking-API.patch @@ -51,11 +51,11 @@ index 87327df6a37668eaf87394b6b049e6d4badec6df..a13c8ddd4a1222e7a16debb61769af37 /** diff --git a/src/main/java/org/bukkit/block/data/BlockData.java b/src/main/java/org/bukkit/block/data/BlockData.java -index a2dc7376b2a3d386b671c894f73389139e0d97bf..26b70af4a1f3db5b17957bfa644e758603f8863c 100644 +index 890a511355dd3f2aa9330fdc72c0fb4b3e44e5cb..54664651f34311e95f6c2dcfd93e58477beda8c2 100644 --- a/src/main/java/org/bukkit/block/data/BlockData.java +++ b/src/main/java/org/bukkit/block/data/BlockData.java -@@ -297,4 +297,14 @@ public interface BlockData extends Cloneable { - @Deprecated(forRemoval = true, since = "1.21") +@@ -295,4 +295,14 @@ public interface BlockData extends Cloneable { + */ float getDestroySpeed(@NotNull ItemStack itemStack, boolean considerEnchants); // Paper end - destroy speed API + diff --git a/patches/api/0425-Add-API-to-get-the-collision-shape-of-a-block-before.patch b/patches/api/0425-Add-API-to-get-the-collision-shape-of-a-block-before.patch index b5d2e36ec2..3bb30787dc 100644 --- a/patches/api/0425-Add-API-to-get-the-collision-shape-of-a-block-before.patch +++ b/patches/api/0425-Add-API-to-get-the-collision-shape-of-a-block-before.patch @@ -6,7 +6,7 @@ Subject: [PATCH] Add API to get the collision shape of a block before it's diff --git a/src/main/java/org/bukkit/block/data/BlockData.java b/src/main/java/org/bukkit/block/data/BlockData.java -index 26b70af4a1f3db5b17957bfa644e758603f8863c..a1ee73254b1389396e7d53f08abe4b3780bd3d0e 100644 +index 54664651f34311e95f6c2dcfd93e58477beda8c2..0ecc54bd810a2805b7209d9433b76743500e45a8 100644 --- a/src/main/java/org/bukkit/block/data/BlockData.java +++ b/src/main/java/org/bukkit/block/data/BlockData.java @@ -205,6 +205,19 @@ public interface BlockData extends Cloneable { diff --git a/patches/server/0436-Add-Destroy-Speed-API.patch b/patches/server/0436-Add-Destroy-Speed-API.patch index 6832fd221a..43e752ecb5 100644 --- a/patches/server/0436-Add-Destroy-Speed-API.patch +++ b/patches/server/0436-Add-Destroy-Speed-API.patch @@ -5,11 +5,40 @@ Subject: [PATCH] Add Destroy Speed API Co-authored-by: Jake Potrebic +diff --git a/src/main/java/net/minecraft/world/entity/ai/attributes/AttributeInstance.java b/src/main/java/net/minecraft/world/entity/ai/attributes/AttributeInstance.java +index 36c943d709c1c0ae49ec0baf0ccf7b6cb9a2ecf3..d28f9e077a50122e86848cfa9db83f6b0e8eef6c 100644 +--- a/src/main/java/net/minecraft/world/entity/ai/attributes/AttributeInstance.java ++++ b/src/main/java/net/minecraft/world/entity/ai/attributes/AttributeInstance.java +@@ -143,20 +143,20 @@ public class AttributeInstance { + double d = this.getBaseValue(); + + for (AttributeModifier attributeModifier : this.getModifiersOrEmpty(AttributeModifier.Operation.ADD_VALUE)) { +- d += attributeModifier.amount(); ++ d += attributeModifier.amount(); // Paper - destroy speed API - diff on change + } + + double e = d; + + for (AttributeModifier attributeModifier2 : this.getModifiersOrEmpty(AttributeModifier.Operation.ADD_MULTIPLIED_BASE)) { +- e += d * attributeModifier2.amount(); ++ e += d * attributeModifier2.amount(); // Paper - destroy speed API - diff on change + } + + for (AttributeModifier attributeModifier3 : this.getModifiersOrEmpty(AttributeModifier.Operation.ADD_MULTIPLIED_TOTAL)) { +- e *= 1.0 + attributeModifier3.amount(); ++ e *= 1.0 + attributeModifier3.amount(); // Paper - destroy speed API - diff on change + } + +- return this.attribute.value().sanitizeValue(e); ++ return attribute.value().sanitizeValue(e); // Paper - destroy speed API - diff on change + } + + private Collection getModifiersOrEmpty(AttributeModifier.Operation operation) { diff --git a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java -index 9953b6b36cbcbfd1756bac478b568ca5700fc898..b318b287572ced45cc3e9f0691a98a037635fbce 100644 +index 9953b6b36cbcbfd1756bac478b568ca5700fc898..33869e725c3b3f2120fa36ca468019a78321e862 100644 --- a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java +++ b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java -@@ -721,4 +721,27 @@ public class CraftBlockData implements BlockData { +@@ -721,4 +721,35 @@ public class CraftBlockData implements BlockData { public BlockState createBlockState() { return CraftBlockStates.getBlockState(this.state, null); } @@ -20,20 +49,171 @@ index 9953b6b36cbcbfd1756bac478b568ca5700fc898..b318b287572ced45cc3e9f0691a98a03 + net.minecraft.world.item.ItemStack nmsItemStack = CraftItemStack.unwrap(itemStack); + float speed = nmsItemStack.getDestroySpeed(this.state); + if (speed > 1.0F && considerEnchants) { -+ final net.minecraft.core.Holder efficiencyHolder = net.minecraft.server.MinecraftServer -+ .getServer() -+ .registryAccess() -+ .registryOrThrow(net.minecraft.core.registries.Registries.ENCHANTMENT) -+ .getHolderOrThrow(net.minecraft.world.item.enchantment.Enchantments.EFFICIENCY); ++ final net.minecraft.core.Holder attribute = net.minecraft.world.entity.ai.attributes.Attributes.MINING_EFFICIENCY; ++ // Logic sourced from AttributeInstance#calculateValue ++ final double initialBaseValue = attribute.value().getDefaultValue(); ++ final org.apache.commons.lang3.mutable.MutableDouble modifiedBaseValue = new org.apache.commons.lang3.mutable.MutableDouble(initialBaseValue); ++ final org.apache.commons.lang3.mutable.MutableDouble baseValMul = new org.apache.commons.lang3.mutable.MutableDouble(1); ++ final org.apache.commons.lang3.mutable.MutableDouble totalValMul = new org.apache.commons.lang3.mutable.MutableDouble(1); + -+ final int enchantLevel = net.minecraft.world.item.enchantment.EnchantmentHelper.getItemEnchantmentLevel( -+ efficiencyHolder, nmsItemStack ++ net.minecraft.world.item.enchantment.EnchantmentHelper.forEachModifier( ++ nmsItemStack, net.minecraft.world.entity.EquipmentSlot.MAINHAND, (attributeHolder, attributeModifier) -> { ++ switch (attributeModifier.operation()) { ++ case ADD_VALUE -> modifiedBaseValue.add(attributeModifier.amount()); ++ case ADD_MULTIPLIED_BASE -> baseValMul.add(attributeModifier.amount()); ++ case ADD_MULTIPLIED_TOTAL -> totalValMul.setValue(totalValMul.doubleValue() * (1D + attributeModifier.amount())); ++ } ++ } + ); -+ if (enchantLevel > 0) { -+ speed += enchantLevel * enchantLevel + 1; -+ } ++ ++ final double actualModifier = modifiedBaseValue.doubleValue() * baseValMul.doubleValue() * totalValMul.doubleValue(); ++ ++ speed += (float) attribute.value().sanitizeValue(actualModifier); + } + return speed; + } + // Paper end - destroy speed API } +diff --git a/src/test/java/io/papermc/paper/block/CraftBlockDataDestroySpeedTest.java b/src/test/java/io/papermc/paper/block/CraftBlockDataDestroySpeedTest.java +new file mode 100644 +index 0000000000000000000000000000000000000000..62aef9abab896491f7806490184fc6899ec36c57 +--- /dev/null ++++ b/src/test/java/io/papermc/paper/block/CraftBlockDataDestroySpeedTest.java +@@ -0,0 +1,137 @@ ++package io.papermc.paper.block; ++ ++import java.util.List; ++import java.util.Optional; ++import net.minecraft.core.Holder; ++import net.minecraft.core.HolderSet; ++import net.minecraft.core.component.DataComponentMap; ++import net.minecraft.core.component.DataComponents; ++import net.minecraft.network.chat.Component; ++import net.minecraft.world.entity.EquipmentSlot; ++import net.minecraft.world.entity.EquipmentSlotGroup; ++import net.minecraft.world.entity.ai.attributes.AttributeInstance; ++import net.minecraft.world.entity.ai.attributes.AttributeModifier; ++import net.minecraft.world.entity.ai.attributes.Attributes; ++import net.minecraft.world.item.Items; ++import net.minecraft.world.item.enchantment.Enchantment; ++import net.minecraft.world.item.enchantment.EnchantmentEffectComponents; ++import net.minecraft.world.item.enchantment.EnchantmentHelper; ++import net.minecraft.world.item.enchantment.ItemEnchantments; ++import net.minecraft.world.item.enchantment.LevelBasedValue; ++import net.minecraft.world.item.enchantment.effects.EnchantmentAttributeEffect; ++import net.minecraft.world.level.block.Blocks; ++import net.minecraft.world.level.block.state.BlockState; ++import org.bukkit.craftbukkit.block.data.CraftBlockData; ++import org.bukkit.craftbukkit.inventory.CraftItemStack; ++import org.bukkit.inventory.ItemStack; ++import org.bukkit.support.AbstractTestingBase; ++import org.bukkit.util.Vector; ++import org.jetbrains.annotations.NotNull; ++import org.junit.jupiter.api.Assertions; ++import org.junit.jupiter.api.Test; ++ ++import static net.minecraft.resources.ResourceLocation.fromNamespaceAndPath; ++ ++/** ++ * CraftBlockData's {@link org.bukkit.craftbukkit.block.data.CraftBlockData#getDestroySpeed(ItemStack, boolean)} ++ * uses a reimplementation of AttributeValue without any map to avoid attribute instance allocation and mutation ++ * for 0 gain. ++ *

++ * This test is responsible for ensuring that said logic emits the expected destroy speed under heavy attribute ++ * modifier use. ++ */ ++public class CraftBlockDataDestroySpeedTest extends AbstractTestingBase { ++ ++ @Test ++ public void testCorrectEnchantmentDestroySpeedComputation() { ++ // Construct fake enchantment that has *all and multiple of* operations ++ final Enchantment speedEnchantment = speedEnchantment(); ++ final BlockState blockStateToMine = Blocks.STONE.defaultBlockState(); ++ ++ final ItemEnchantments.Mutable mutable = new ItemEnchantments.Mutable(ItemEnchantments.EMPTY); ++ mutable.set(Holder.direct(speedEnchantment), 1); ++ ++ final net.minecraft.world.item.ItemStack itemStack = new net.minecraft.world.item.ItemStack(Items.DIAMOND_PICKAXE); ++ itemStack.set(DataComponents.ENCHANTMENTS, mutable.toImmutable()); ++ ++ // Compute expected value by running the entire attribute instance chain ++ final AttributeInstance dummyInstance = new AttributeInstance(Attributes.MINING_EFFICIENCY, $ -> { ++ }); ++ EnchantmentHelper.forEachModifier(itemStack, EquipmentSlot.MAINHAND, (attributeHolder, attributeModifier) -> { ++ if (attributeHolder.is(Attributes.MINING_EFFICIENCY)) dummyInstance.addTransientModifier(attributeModifier); ++ }); ++ ++ final double toolSpeed = itemStack.getDestroySpeed(blockStateToMine); ++ final double expectedSpeed = toolSpeed <= 1.0F ? toolSpeed : toolSpeed + dummyInstance.getValue(); ++ ++ // API stack + computation ++ final CraftItemStack craftMirror = CraftItemStack.asCraftMirror(itemStack); ++ final CraftBlockData data = CraftBlockData.createData(blockStateToMine); ++ final float actualSpeed = data.getDestroySpeed(craftMirror, true); ++ ++ Assertions.assertEquals(expectedSpeed, actualSpeed, Vector.getEpsilon()); ++ } ++ ++ /** ++ * Complex enchantment that holds attribute modifiers for the mining efficiency. ++ * The enchantment holds 2 of each operation to also ensure that such behaviour works correctly. ++ * ++ * @return the enchantment. ++ */ ++ private static @NotNull Enchantment speedEnchantment() { ++ return new Enchantment( ++ Component.empty(), ++ new Enchantment.EnchantmentDefinition( ++ HolderSet.empty(), ++ Optional.empty(), ++ 0, 0, ++ Enchantment.constantCost(0), ++ Enchantment.constantCost(0), ++ 0, ++ List.of(EquipmentSlotGroup.ANY) ++ ), ++ HolderSet.empty(), ++ DataComponentMap.builder() ++ .set(EnchantmentEffectComponents.ATTRIBUTES, List.of( ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "base1"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.constant(1), ++ AttributeModifier.Operation.ADD_VALUE ++ ), ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "base2"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.perLevel(3), ++ AttributeModifier.Operation.ADD_VALUE ++ ), ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "base-mul1"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.perLevel(7), ++ AttributeModifier.Operation.ADD_MULTIPLIED_BASE ++ ), ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "base-mul2"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.constant(10), ++ AttributeModifier.Operation.ADD_MULTIPLIED_BASE ++ ), ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "total-mul1"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.constant(.2f), ++ AttributeModifier.Operation.ADD_MULTIPLIED_TOTAL ++ ), ++ new EnchantmentAttributeEffect( ++ fromNamespaceAndPath("paper", "total-mul2"), ++ Attributes.MINING_EFFICIENCY, ++ LevelBasedValue.constant(-.5F), ++ AttributeModifier.Operation.ADD_MULTIPLIED_TOTAL ++ ) ++ )) ++ .build() ++ ); ++ } ++ ++} diff --git a/patches/server/0736-Block-Ticking-API.patch b/patches/server/0736-Block-Ticking-API.patch index 3da9c64590..9e5247f35f 100644 --- a/patches/server/0736-Block-Ticking-API.patch +++ b/patches/server/0736-Block-Ticking-API.patch @@ -46,10 +46,10 @@ index 6d10396347b69d9243ab902ecc68ede93fa17b7d..af219df5267589300f0ad1d30fa5c81a // Paper end } diff --git a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java -index b318b287572ced45cc3e9f0691a98a037635fbce..53dddaf1fb608312991739d488b8cd2dadc58e22 100644 +index 33869e725c3b3f2120fa36ca468019a78321e862..64ddd6d1c40dc91b6e7fc3118403415bb4533d97 100644 --- a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java +++ b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java -@@ -744,4 +744,11 @@ public class CraftBlockData implements BlockData { +@@ -752,4 +752,11 @@ public class CraftBlockData implements BlockData { return speed; } // Paper end - destroy speed API diff --git a/patches/server/0906-Add-API-to-get-the-collision-shape-of-a-block-before.patch b/patches/server/0906-Add-API-to-get-the-collision-shape-of-a-block-before.patch index ea30c04785..f0fcc8b9eb 100644 --- a/patches/server/0906-Add-API-to-get-the-collision-shape-of-a-block-before.patch +++ b/patches/server/0906-Add-API-to-get-the-collision-shape-of-a-block-before.patch @@ -6,7 +6,7 @@ Subject: [PATCH] Add API to get the collision shape of a block before it's diff --git a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java -index 53dddaf1fb608312991739d488b8cd2dadc58e22..17933c51abf657335fd449635f198c6802adf14c 100644 +index 64ddd6d1c40dc91b6e7fc3118403415bb4533d97..0daa0bf7e56aa7228d89867500cb780b37f06541 100644 --- a/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java +++ b/src/main/java/org/bukkit/craftbukkit/block/data/CraftBlockData.java @@ -679,6 +679,20 @@ public class CraftBlockData implements BlockData {