Change Enchantment API for data-driven enchantments

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/v1/EnchantingContext.java

@@ -16,41 +16,25 @@
 
 package net.fabricmc.fabric.api.item.v1;
 
-import net.minecraft.enchantment.EnchantmentHelper;
+import net.minecraft.enchantment.Enchantment;
 import net.minecraft.item.ItemStack;
-import net.minecraft.resource.featuretoggle.FeatureSet;
-import net.minecraft.util.math.random.Random;
 
-/*
- * There is one context for each vanilla call to Enchantment#isAcceptableItem. The reason why RANDOM_ENCHANTMENT
- * feels like a kitchen sink is because it corresponds to the one in EnchantmentHelper, which is shared across multiple
- * uses.
- *
- * This also gets in the way of adding further context (nullable Player and BlockPos have been suggested
- * in the past). It's not impossible to do so, but a probably a bit more brittle.
- */
 /**
- * An enum that describes the various contexts in which the game checks whether an enchantment can be applied to an item.
+ * An enum that describes the contexts in which the game checks whether an enchantment can be applied to an item.
  */
 public enum EnchantingContext {
 	/**
-	 * When generating a random enchantment for the item. This includes the enchanting table, random
-	 * mob equipment, and the {@code enchant_with_levels} loot function.
+	 * When checking if an item is <em>acceptable</em> for a given enchantment, i.e if the item should be able to bear
+	 * that enchantment. This includes anvils, the {@code enchant_randomly} loot function, and the {@code /enchant} command.
 	 *
-	 * @see EnchantmentHelper#generateEnchantments(FeatureSet, Random, ItemStack, int, boolean)
-	 */
-	RANDOM_ENCHANTMENT,
-	/**
-	 * When trying to apply an enchantment in an anvil.
-	 */
-	ANVIL,
-	/**
-	 * When using the {@code /enchant} command.
+	 * @see Enchantment#isAcceptableItem(ItemStack)
 	 */
-	ENCHANT_COMMAND,
+	ACCEPTABLE,
 	/**
-	 * When randomly enchanting an item using the {@code enchant_randomly} loot function without a list of enchantments
-	 * to choose from.
+	 * When checking for an enchantment's <em>primary</em> items. This includes enchanting in an enchanting table, random
+	 * mob equipment, and the {@code enchant_with_levels} loot function.
+	 *
+	 * @see Enchantment#isPrimaryItem(ItemStack)
 	 */
-	LOOT_RANDOM_ENCHANTMENT
+	PRIMARY
 }

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/v1/EnchantmentEvents.java

@@ -37,9 +37,9 @@ private EnchantmentEvents() { }
 	 * where 'external' means either vanilla or from another mod. For instance, a mod might allow enchanting a pickaxe
 	 * with Sharpness (and only Sharpness) under certain specific conditions.</p>
 	 *
-	 * <p>To modify the behavior of your own modded <em>enchantments</em>, use {@link Enchantment#isAcceptableItem(ItemStack)} instead.
-	 * To modify the behavior of your own modded <em>items</em>, use {@link FabricItem#canBeEnchantedWith(ItemStack, Enchantment, EnchantingContext)} instead.
-	 * Note that this event triggers <em>before</em> {@link FabricItem#canBeEnchantedWith(ItemStack, Enchantment, EnchantingContext)},
+	 * <p>To modify the behavior of your own modded <em>enchantments</em>, specify a custom tag for {@link Enchantment.Definition#supportedItems()} instead.
+	 * To modify the behavior of your own modded <em>items</em>, add to the applicable tags instead, when that suffices.
+	 * Note that this event triggers <em>before</em> {@link FabricItem#canBeEnchantedWith(ItemStack, RegistryEntry, EnchantingContext)},
 	 * and that method will only be called if no listeners override it.</p>
 	 *
 	 * <p>Note that allowing an enchantment using this event does not guarantee the item will receive that enchantment,

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/v1/FabricItem.java

@@ -126,7 +126,9 @@ default ItemStack getRecipeRemainder(ItemStack stack) {
 	 * @return whether the enchantment is allowed to apply to the stack
 	 */
 	default boolean canBeEnchantedWith(ItemStack stack, RegistryEntry<Enchantment> enchantment, EnchantingContext context) {
-		return enchantment.value().isAcceptableItem(stack);
+		return context == EnchantingContext.PRIMARY
+				? enchantment.value().isPrimaryItem(stack)
+				: enchantment.value().isAcceptableItem(stack);
 	}
 
 	/**

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/api/item/v1/FabricItemStack.java

@@ -45,7 +45,8 @@ default ItemStack getRecipeRemainder() {
 	 * Determines whether this {@link ItemStack} can be enchanted with the given {@link Enchantment}.
 	 *
 	 * <p>When checking whether an enchantment can be applied to an {@link ItemStack}, use this method instead of
-	 * {@link Enchantment#isAcceptableItem(ItemStack)}</p>
+	 * {@link Enchantment#isAcceptableItem(ItemStack)} or {@link Enchantment#isPrimaryItem(ItemStack)}, with the appropriate
+	 * {@link EnchantingContext}.</p>
 	 *
 	 * @param enchantment the enchantment to check
 	 * @param context the context in which the enchantment is being checked

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/AnvilScreenHandlerMixin.java

@@ -47,6 +47,6 @@ abstract class AnvilScreenHandlerMixin extends ForgingScreenHandler {
 			)
 	)
 	private boolean callAllowEnchantingEvent(Enchantment instance, ItemStack stack, @Local RegistryEntry<Enchantment> registryEntry) {
-		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.ANVIL);
+		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.ACCEPTABLE);
 	}
 }

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/EnchantCommandMixin.java

@@ -38,6 +38,6 @@ abstract class EnchantCommandMixin {
 			at = @At(value = "INVOKE", target = "Lnet/minecraft/enchantment/Enchantment;isAcceptableItem(Lnet/minecraft/item/ItemStack;)Z")
 	)
 	private static boolean callAllowEnchantingEvent(Enchantment instance, ItemStack stack, ServerCommandSource source, Collection<? extends Entity> targets, RegistryEntry<Enchantment> enchantment) {
-		return stack.canBeEnchantedWith(enchantment, EnchantingContext.ENCHANT_COMMAND);
+		return stack.canBeEnchantedWith(enchantment, EnchantingContext.ACCEPTABLE);
 	}
 }

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/EnchantRandomlyLootFunctionMixin.java

@@ -34,6 +34,6 @@ abstract class EnchantRandomlyLootFunctionMixin {
 			at = @At(value = "INVOKE", target = "Lnet/minecraft/enchantment/Enchantment;isAcceptableItem(Lnet/minecraft/item/ItemStack;)Z")
 	)
 	private static boolean callAllowEnchantingEvent(Enchantment enchantment, ItemStack stack, boolean bl, ItemStack itemStack, RegistryEntry<Enchantment> registryEntry) {
-		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.LOOT_RANDOM_ENCHANTMENT);
+		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.ACCEPTABLE);
 	}
 }

fabric-item-api-v1/src/main/java/net/fabricmc/fabric/mixin/item/EnchantmentHelperMixin.java

@@ -34,6 +34,6 @@ abstract class EnchantmentHelperMixin {
 			at = @At(value = "INVOKE", target = "Lnet/minecraft/enchantment/Enchantment;isPrimaryItem(Lnet/minecraft/item/ItemStack;)Z")
 	)
 	private static boolean useCustomEnchantingChecks(Enchantment instance, ItemStack stack, ItemStack itemStack, boolean bl, RegistryEntry<Enchantment> registryEntry) {
-		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.RANDOM_ENCHANTMENT);
+		return stack.canBeEnchantedWith(registryEntry, EnchantingContext.PRIMARY);
 	}
 }

fabric-item-api-v1/src/testmod/java/net/fabricmc/fabric/test/item/CustomDamageTest.java

@@ -30,6 +30,7 @@
 import net.minecraft.registry.Registries;
 import net.minecraft.registry.Registry;
 import net.minecraft.registry.entry.RegistryEntry;
+import net.minecraft.registry.tag.EnchantmentTags;
 import net.minecraft.text.Text;
 import net.minecraft.util.Identifier;
 import net.minecraft.util.dynamic.Codecs;
@@ -63,17 +64,7 @@ public void onInitialize() {
 		FuelRegistry.INSTANCE.add(WEIRD_PICK, 200);
 		FabricBrewingRecipeRegistryBuilder.BUILD.register(builder -> builder.registerPotionRecipe(Potions.WATER, WEIRD_PICK, Potions.AWKWARD));
 		EnchantmentEvents.ALLOW_ENCHANTING.register(((enchantment, target, enchantingContext) -> {
-			// TODO 1.21 this is clearly wrong
-			boolean hasSilkTouch = false;
-
-			for (RegistryEntry<Enchantment> entry : EnchantmentHelper.getEnchantments(target).getEnchantments()) {
-				if (entry.getKey().orElse(null) == Enchantments.SILK_TOUCH) {
-					hasSilkTouch = true;
-					break;
-				}
-			}
-
-			if (target.isOf(Items.DIAMOND_PICKAXE) && enchantment == Enchantments.SHARPNESS && hasSilkTouch) {
+			if (target.isOf(Items.DIAMOND_PICKAXE) && enchantment.matchesKey(Enchantments.SHARPNESS) && EnchantmentHelper.hasAnyEnchantmentsIn(target, EnchantmentTags.MINING_EXCLUSIVE_SET)) {
 				return TriState.TRUE;
 			}
 
@@ -106,8 +97,8 @@ public ItemStack getRecipeRemainder(ItemStack stack) {
 
 		@Override
 		public boolean canBeEnchantedWith(ItemStack stack, RegistryEntry<Enchantment> enchantment, EnchantingContext context) {
-			return context == EnchantingContext.ANVIL && enchantment == Enchantments.FIRE_ASPECT
-				|| enchantment != Enchantments.FORTUNE && super.canBeEnchantedWith(stack, enchantment, context);
+			return context == EnchantingContext.ACCEPTABLE && enchantment.matchesKey(Enchantments.FIRE_ASPECT)
+				|| !enchantment.matchesKey(Enchantments.FORTUNE) && super.canBeEnchantedWith(stack, enchantment, context);
 		}
 	}
 }