neoforged / NeoForge

Neo Modding API for Minecraft, based on Forge
https://projects.neoforged.net/neoforged/neoforge
Other
1.21k stars 177 forks source link

"isPrimaryItemFor" is never called during Anvil operations #1397

Closed vectorwing closed 2 months ago

vectorwing commented 2 months ago

Minecraft Version: 1.21

NeoForge Version: 21.0.145

Logs: Not relevant.

Steps to Reproduce:

  1. Create and register an item whose class extends Item, and overrides isPrimaryItemFor;
  2. Write a custom enchanting rule inside isPrimaryItemFor. Example: If the enchantment fits a Diamond Sword, return true;
  3. Launch the game;
  4. Place and use an Anvil, then attempt to apply a valid enchantment to the item using an Enchanted Book.

Description of issue:

NeoForge introduces an override called isPrimaryItemFor to Item classes, which allows modders to customize enchantment compatibility beyond the usage of tags. This is very versatile, because it lets us include/exclude in a way that some tags won't allow, such as enchantable/sword being tied to many broad enchantments.

However, this method is not called at any time when using the Anvil, only within the Enchanting Table. This causes both workstations to apply enchantments inconsistently, with the Anvil only consulting tags. What can be applied in one cannot be applied in the other.

This felt like a bug/oversight to me, so I thought of reporting it just in case.

Shadows-of-Fire commented 2 months ago

isPrimaryItemFor is intentionally not used by the anvil, since the anvil only checks for supported items - though we may need to introduce a isSupportedItemFor hook as well to handle this separation.

Previously we just had canApplyAtEnchantingTable, which for whatever reason was then called by canEnchant and subsequently used by the anvil, so the name of that old method was a substantial misnomer.

neoforged-releases[bot] commented 2 months ago

🚀 This issue has been resolved in NeoForge version 21.1.4, as part of #1412.