Fix tool attributes mining levels ()

* Decoupled effective checks and mining speed

* Fixed typo

* Bump major for method sig change

* Fix semvar

* Re-add original API method

* Extend test mod

* Fix bug discovered by testing

* Change current to vanillaResult, update doc for postProcessMiningSpeed

* Remove </p> tag

* Add vanilla tests and rename vars
This commit is contained in:
Snakefangox 2020-07-25 03:24:50 +12:00 committed by GitHub
parent 5d32f58344
commit d21d463561
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 172 additions and 56 deletions
fabric-tool-attribute-api-v1

View file

@ -1,5 +1,5 @@
archivesBaseName = "fabric-tool-attribute-api-v1"
version = getSubprojectVersion(project, "1.1.4")
version = getSubprojectVersion(project, "1.2.0")
dependencies {
compile project(path: ':fabric-api-base', configuration: 'dev')

View file

@ -68,7 +68,7 @@ public interface DynamicAttributeTool {
* @param stack The item stack being used to mine the block
* @param user The current user of the tool, or null if there isn't any
* @return The mining speed multiplier of the item. 8.0 is equal to a diamond pick.
* @deprecated Use {@link #getMiningLevel(Tag, BlockState, ItemStack, LivingEntity)} to detect tag and block.
* @deprecated Use {@link #getMiningSpeedMultiplier(Tag, BlockState, ItemStack, LivingEntity)} to detect tag and block.
*/
// nullable on user once we have an official @Nullable annotation in
@Deprecated
@ -91,8 +91,9 @@ public interface DynamicAttributeTool {
}
/**
* Post process the mining speed, can be used to change tool speed regardless if the tool is effective.
* Useful if you want to change the mining speed even if it is not effective.
* Post process the mining speed, this takes place after the mining speed has been calculated.
*
* <p>This allows bypassing the regular computation formula.
*
* @param tag The tool tag the item stack is handled by
* @param state The block to mine

View file

@ -36,7 +36,20 @@ public final class ToolManager {
* @return whether the tool is effective
*/
public static boolean handleIsEffectiveOn(BlockState state, ItemStack stack, /* @Nullable */ LivingEntity user) {
return stack.isEffectiveOn(state) || handleIsEffectiveOnIgnoresVanilla(state, stack, user);
return stack.isEffectiveOn(state) || handleIsEffectiveOnIgnoresVanilla(state, stack, user, false);
}
/**
* Handles if the tool is effective on a block, ignores vanilla tools on vanilla blocks.
*
* @param state the block state to break
* @param stack the item stack involved with breaking the block
* @param user the user involved in breaking the block, null if not applicable.
* @param vanillaResult whether the tool is considered effective by vanilla
* @return whether the tool is effective
*/
public static boolean handleIsEffectiveOnIgnoresVanilla(BlockState state, ItemStack stack, /* @Nullable */ LivingEntity user, boolean vanillaResult) {
return ToolManagerImpl.handleIsEffectiveOnIgnoresVanilla(state, stack, user, vanillaResult);
}
/**
@ -48,7 +61,7 @@ public final class ToolManager {
* @return whether the tool is effective
*/
public static boolean handleIsEffectiveOnIgnoresVanilla(BlockState state, ItemStack stack, /* @Nullable */ LivingEntity user) {
return ToolManagerImpl.handleIsEffectiveOnIgnoresVanilla(state, stack, user);
return ToolManagerImpl.handleIsEffectiveOnIgnoresVanilla(state, stack, user, false);
}
/**

View file

@ -168,7 +168,7 @@ public final class ToolManagerImpl {
* Hook for ItemStack.isEffectiveOn and similar methods.
*/
//TODO: nullable on user once we have an official @Nullable annotation in
public static boolean handleIsEffectiveOnIgnoresVanilla(BlockState state, ItemStack stack, LivingEntity user) {
public static boolean handleIsEffectiveOnIgnoresVanilla(BlockState state, ItemStack stack, LivingEntity user, boolean vanillaResult) {
for (Map.Entry<Tag<Item>, Event<ToolHandler>> eventEntry : HANDLER_MAP.entrySet()) {
if (stack.getItem().isIn(eventEntry.getKey())) {
ActionResult effective = eventEntry.getValue().invoker().isEffectiveOn(eventEntry.getKey(), state, stack, user);
@ -180,7 +180,7 @@ public final class ToolManagerImpl {
EntryImpl entry = (EntryImpl) entryNullable(state.getBlock());
return entry != null && entry.defaultValue.get();
return (entry != null && entry.defaultValue.get()) || (entry == null && vanillaResult);
}
public static float handleBreakingSpeedIgnoresVanilla(BlockState state, ItemStack stack, /* @Nullable */ LivingEntity user) {
@ -190,33 +190,25 @@ public final class ToolManagerImpl {
for (Map.Entry<Tag<Item>, Event<ToolHandler>> eventEntry : HANDLER_MAP.entrySet()) {
if (stack.getItem().isIn(eventEntry.getKey())) {
ActionResult effective = eventEntry.getValue().invoker().isEffectiveOn(eventEntry.getKey(), state, stack, user);
TypedActionResult<Float> speedMultiplier = Objects.requireNonNull(eventEntry.getValue().invoker().getMiningSpeedMultiplier(eventEntry.getKey(), state, stack, user));
if (effective.isAccepted()) {
TypedActionResult<Float> speedMultiplier = Objects.requireNonNull(eventEntry.getValue().invoker().getMiningSpeedMultiplier(eventEntry.getKey(), state, stack, user));
if (speedMultiplier.getResult().isAccepted()) {
handled = true;
if (speedMultiplier.getResult().isAccepted()) {
handled = true;
if (speedMultiplier.getValue() > breakingSpeed) {
breakingSpeed = speedMultiplier.getValue();
handledTag = eventEntry.getKey();
}
if (speedMultiplier.getValue() > breakingSpeed) {
breakingSpeed = speedMultiplier.getValue();
handledTag = eventEntry.getKey();
}
}
effective = general().invoker().isEffectiveOn(eventEntry.getKey(), state, stack, user);
speedMultiplier = Objects.requireNonNull(general().invoker().getMiningSpeedMultiplier(eventEntry.getKey(), state, stack, user));
if (effective.isAccepted()) {
TypedActionResult<Float> speedMultiplier = Objects.requireNonNull(general().invoker().getMiningSpeedMultiplier(eventEntry.getKey(), state, stack, user));
if (speedMultiplier.getResult().isAccepted()) {
handled = true;
if (speedMultiplier.getResult().isAccepted()) {
handled = true;
if (speedMultiplier.getValue() > breakingSpeed) {
breakingSpeed = speedMultiplier.getValue();
handledTag = eventEntry.getKey();
}
if (speedMultiplier.getValue() > breakingSpeed) {
breakingSpeed = speedMultiplier.getValue();
handledTag = eventEntry.getKey();
}
}
}

View file

@ -53,8 +53,12 @@ public class ModdedToolsModdedBlocksToolHandler implements ToolManagerImpl.ToolH
@Override
public TypedActionResult<Float> getMiningSpeedMultiplier(Tag<Item> tag, BlockState state, ItemStack stack, LivingEntity user) {
if (stack.getItem() instanceof DynamicAttributeTool) {
float multiplier = ((DynamicAttributeTool) stack.getItem()).getMiningSpeedMultiplier(tag, state, stack, user);
if (multiplier != 1.0F) return TypedActionResult.success(multiplier);
ToolManagerImpl.Entry entry = ToolManagerImpl.entryNullable(state.getBlock());
if (entry != null && entry.getMiningLevel(tag) > 0) {
float multiplier = ((DynamicAttributeTool) stack.getItem()).getMiningSpeedMultiplier(tag, state, stack, user);
if (multiplier != 1.0F) return TypedActionResult.success(multiplier);
}
}
return TypedActionResult.pass(1.0F);

View file

@ -63,8 +63,7 @@ public class ModdedToolsVanillaBlocksToolHandler implements ToolManagerImpl.Tool
if (miningLevel < 0) return ActionResult.PASS;
ToolItem vanillaItem = getVanillaItem(miningLevel);
boolean effective = vanillaItem.isEffectiveOn(state) || vanillaItem.getMiningSpeedMultiplier(new ItemStack(vanillaItem), state) != 1.0F;
return effective ? ActionResult.SUCCESS : ActionResult.PASS;
return vanillaItem.isEffectiveOn(state) ? ActionResult.SUCCESS : ActionResult.PASS;
}
return ActionResult.PASS;

View file

@ -50,9 +50,9 @@ public class ShearsVanillaBlocksToolHandler implements ToolManagerImpl.ToolHandl
if (!(stack.getItem() instanceof DynamicAttributeTool)) {
if (!(stack.getItem() instanceof ShearsItem)) {
return vanillaItem.isEffectiveOn(state) || vanillaItem.getMiningSpeedMultiplier(new ItemStack(vanillaItem), state) != 1.0F ? ActionResult.SUCCESS : ActionResult.PASS;
return vanillaItem.isEffectiveOn(state) ? ActionResult.SUCCESS : ActionResult.PASS;
} else {
return stack.getItem().isEffectiveOn(state) || stack.getItem().getMiningSpeedMultiplier(stack, state) != 1.0F ? ActionResult.SUCCESS : ActionResult.PASS;
return stack.getItem().isEffectiveOn(state) ? ActionResult.SUCCESS : ActionResult.PASS;
}
}

View file

@ -54,8 +54,12 @@ public class VanillaToolsModdedBlocksToolHandler implements ToolManagerImpl.Tool
@Override
public TypedActionResult<Float> getMiningSpeedMultiplier(Tag<Item> tag, BlockState state, ItemStack stack, LivingEntity user) {
if (!(stack.getItem() instanceof DynamicAttributeTool)) {
float multiplier = stack.getItem() instanceof ToolItem ? ((ToolItem) stack.getItem()).getMaterial().getMiningSpeedMultiplier() : stack.getItem().getMiningSpeedMultiplier(stack, state);
if (multiplier != 1.0F) return TypedActionResult.success(multiplier);
ToolManagerImpl.Entry entry = ToolManagerImpl.entryNullable(state.getBlock());
if (entry != null && entry.getMiningLevel(tag) >= 0 && tag.contains(stack.getItem())) {
float multiplier = stack.getItem() instanceof ToolItem ? ((ToolItem) stack.getItem()).getMaterial().getMiningSpeedMultiplier() : stack.getItem().getMiningSpeedMultiplier(stack, state);
if (multiplier != 1.0F) return TypedActionResult.success(multiplier);
}
}
return TypedActionResult.pass(1.0F);

View file

@ -42,9 +42,7 @@ public abstract class MixinItemStack {
@Inject(at = @At("RETURN"), method = "isEffectiveOn", cancellable = true)
public void isEffectiveOn(BlockState state, CallbackInfoReturnable<Boolean> info) {
if (!info.getReturnValueZ()) {
info.setReturnValue(ToolManager.handleIsEffectiveOnIgnoresVanilla(state, (ItemStack) (Object) this, null));
}
info.setReturnValue(ToolManager.handleIsEffectiveOnIgnoresVanilla(state, (ItemStack) (Object) this, null, info.getReturnValueZ()));
}
@Inject(at = @At("RETURN"), method = "getMiningSpeedMultiplier", cancellable = true)

View file

@ -18,11 +18,15 @@ package net.fabricmc.fabric.test.tool.attribute;
import net.minecraft.block.Block;
import net.minecraft.block.BlockState;
import net.minecraft.block.Blocks;
import net.minecraft.block.Material;
import net.minecraft.block.MaterialColor;
import net.minecraft.entity.LivingEntity;
import net.minecraft.item.BlockItem;
import net.minecraft.item.Item;
import net.minecraft.item.ItemStack;
import net.minecraft.item.Items;
import net.minecraft.item.ToolItem;
import net.minecraft.server.MinecraftServer;
import net.minecraft.sound.BlockSoundGroup;
import net.minecraft.tag.Tag;
@ -30,51 +34,113 @@ import net.minecraft.util.Identifier;
import net.minecraft.util.registry.Registry;
import net.fabricmc.api.ModInitializer;
import net.fabricmc.fabric.api.event.server.ServerTickCallback;
import net.fabricmc.fabric.api.object.builder.v1.block.FabricBlockSettings;
import net.fabricmc.fabric.api.object.builder.v1.block.FabricMaterialBuilder;
import net.fabricmc.fabric.api.tool.attribute.v1.DynamicAttributeTool;
import net.fabricmc.fabric.api.tool.attribute.v1.FabricToolTags;
import net.fabricmc.fabric.api.event.server.ServerTickCallback;
public class ToolAttributeTest implements ModInitializer {
private boolean hasValidatedTags = false;
private static final float DEFAULT_BREAK_SPEED = 1.0F;
private static final float TOOL_BREAK_SPEED = 10.0F;
private boolean hasValidated = false;
Block gravelBlock;
Block stoneBlock;
Item testShovel;
Item testPickaxe;
@Override
public void onInitialize() {
// Register a custom shovel that has a mining level of 2 (iron) dynamically.
Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "test_shovel"), new TestShovel(new Item.Settings()));
testShovel = Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "test_shovel"), new TestTool(new Item.Settings(), FabricToolTags.SHOVELS));
//Register a custom pickaxe that has a mining level of 2 (iron) dynamically.
testPickaxe = Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "test_pickaxe"), new TestTool(new Item.Settings(), FabricToolTags.PICKAXES));
// Register a block that requires a shovel that is as strong or stronger than an iron one.
Block block = Registry.register(Registry.BLOCK, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_block"),
gravelBlock = Registry.register(Registry.BLOCK, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_gravel_block"),
new Block(FabricBlockSettings.of(new FabricMaterialBuilder(MaterialColor.SAND).build(), MaterialColor.STONE)
.breakByTool(FabricToolTags.SHOVELS, 2)
.requiresTool()
.strength(0.6F)
.sounds(BlockSoundGroup.GRAVEL)));
Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_block"), new BlockItem(block, new Item.Settings()));
Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_gravel_block"), new BlockItem(gravelBlock, new Item.Settings()));
// Register a block that requires a pickaxe that is as strong or stronger than an iron one.
stoneBlock = Registry.register(Registry.BLOCK, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_stone_block"),
new Block(FabricBlockSettings.of(Material.STONE, MaterialColor.STONE)
.breakByTool(FabricToolTags.PICKAXES, 2)
.requiresTool()
.strength(0.6F)
.sounds(BlockSoundGroup.STONE)));
Registry.register(Registry.ITEM, new Identifier("fabric-tool-attribute-api-v1-testmod", "hardened_stone_block"), new BlockItem(stoneBlock, new Item.Settings()));
ServerTickCallback.EVENT.register(this::validateTags);
ServerTickCallback.EVENT.register(this::validate);
}
private void validateTags(MinecraftServer server) {
if (hasValidatedTags) {
private void validate(MinecraftServer server) {
if (hasValidated) {
return;
}
hasValidatedTags = true;
hasValidated = true;
if (FabricToolTags.PICKAXES.values().isEmpty()) {
throw new AssertionError("Failed to load tool tags");
}
//Test we haven't broken vanilla behavior
testToolOnBlock(new ItemStack(Items.STONE_PICKAXE), Blocks.GRAVEL, false, 1.0F);
testToolOnBlock(new ItemStack(Items.IRON_PICKAXE), Blocks.STONE, true, ((ToolItem) Items.IRON_PICKAXE).getMaterial().getMiningSpeedMultiplier());
testToolOnBlock(new ItemStack(Items.IRON_PICKAXE), Blocks.OBSIDIAN, false, ((ToolItem) Items.IRON_PICKAXE).getMaterial().getMiningSpeedMultiplier());
testToolOnBlock(new ItemStack(Items.STONE_SHOVEL), Blocks.STONE, false, 1.0F);
testToolOnBlock(new ItemStack(Items.STONE_SHOVEL), Blocks.GRAVEL, false, ((ToolItem) Items.STONE_SHOVEL).getMaterial().getMiningSpeedMultiplier());
//Test vanilla tools don't bypass fabric mining levels
testToolOnBlock(new ItemStack(Items.STONE_PICKAXE), stoneBlock, false, ((ToolItem) Items.STONE_PICKAXE).getMaterial().getMiningSpeedMultiplier());
testToolOnBlock(new ItemStack(Items.IRON_PICKAXE), stoneBlock, true, ((ToolItem) Items.IRON_PICKAXE).getMaterial().getMiningSpeedMultiplier());
testToolOnBlock(new ItemStack(Items.STONE_SHOVEL), gravelBlock, false, ((ToolItem) Items.STONE_SHOVEL).getMaterial().getMiningSpeedMultiplier());
testToolOnBlock(new ItemStack(Items.IRON_SHOVEL), gravelBlock, true, ((ToolItem) Items.IRON_SHOVEL).getMaterial().getMiningSpeedMultiplier());
//Test vanilla tools respect fabric mining tags
testToolOnBlock(new ItemStack(Items.IRON_PICKAXE), gravelBlock, false, DEFAULT_BREAK_SPEED);
testToolOnBlock(new ItemStack(Items.IRON_SHOVEL), stoneBlock, false, DEFAULT_BREAK_SPEED);
//Test dynamic tools don't bypass mining level
testToolOnBlock(new ItemStack(testPickaxe), Blocks.OBSIDIAN, false, TOOL_BREAK_SPEED);
//Test dynamic tools respect fabric mining tags
testToolOnBlock(new ItemStack(testPickaxe), gravelBlock, false, DEFAULT_BREAK_SPEED);
testToolOnBlock(new ItemStack(testShovel), stoneBlock, false, DEFAULT_BREAK_SPEED);
//Test dynamic tools on vanilla blocks
testToolOnBlock(new ItemStack(testShovel), Blocks.STONE, false, DEFAULT_BREAK_SPEED);
testToolOnBlock(new ItemStack(testShovel), Blocks.GRAVEL, false, TOOL_BREAK_SPEED);
testToolOnBlock(new ItemStack(testPickaxe), Blocks.GRAVEL, false, DEFAULT_BREAK_SPEED);
testToolOnBlock(new ItemStack(testPickaxe), Blocks.STONE, true, TOOL_BREAK_SPEED);
}
private static class TestShovel extends Item implements DynamicAttributeTool {
private TestShovel(Settings settings) {
private void testToolOnBlock(ItemStack item, Block block, boolean inEffective, float inSpeed) {
boolean effective = item.isEffectiveOn(block.getDefaultState());
float speed = item.getMiningSpeedMultiplier(block.getDefaultState());
if (inEffective != effective) {
throw new AssertionError("Effective check incorrect for " + Registry.ITEM.getId(item.getItem()) + " breaking " + Registry.BLOCK.getId(block) + " got " + effective);
} else if (inSpeed != speed) {
throw new AssertionError("Speed check incorrect for " + Registry.ITEM.getId(item.getItem()) + " breaking " + Registry.BLOCK.getId(block) + " got " + speed);
}
}
private static class TestTool extends Item implements DynamicAttributeTool {
final Tag<Item> toolType;
private TestTool(Settings settings, Tag<Item> toolType) {
super(settings);
this.toolType = toolType;
}
@Override
public int getMiningLevel(Tag<Item> tag, BlockState state, ItemStack stack, LivingEntity user) {
if (tag.equals(FabricToolTags.SHOVELS)) {
if (tag.equals(toolType)) {
return 2;
}
@ -83,11 +149,11 @@ public class ToolAttributeTest implements ModInitializer {
@Override
public float getMiningSpeedMultiplier(Tag<Item> tag, BlockState state, ItemStack stack, LivingEntity user) {
if (tag.equals(FabricToolTags.SHOVELS)) {
return 10.0F;
if (tag.equals(toolType)) {
return TOOL_BREAK_SPEED;
}
return 1.0F;
return DEFAULT_BREAK_SPEED;
}
}
}

View file

@ -0,0 +1,5 @@
{
"variants": {
"": { "model": "block/stone" }
}
}

View file

@ -0,0 +1,6 @@
{
"parent": "item/generated",
"textures": {
"layer0": "item/apple"
}
}

View file

@ -0,0 +1,19 @@
{
"type": "minecraft:block",
"pools": [
{
"rolls": 1,
"entries": [
{
"type": "minecraft:item",
"name": "fabric-tool-attribute-api-v1-testmod:hardened_gravel_block"
}
],
"conditions": [
{
"condition": "minecraft:survives_explosion"
}
]
}
]
}

View file

@ -6,7 +6,7 @@
"entries": [
{
"type": "minecraft:item",
"name": "fabric-tool-attribute-api-v1-testmod:hardened_block"
"name": "fabric-tool-attribute-api-v1-testmod:hardened_stone_block"
}
],
"conditions": [

View file

@ -0,0 +1,6 @@
{
"replace": false,
"values": [
"fabric-tool-attribute-api-v1-testmod:test_pickaxe"
]
}

View file

@ -3,4 +3,4 @@
"values": [
"fabric-tool-attribute-api-v1-testmod:test_shovel"
]
}
}