Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move various settings from non vanilla custom item data to custom item data and allow use in JSON mappings #4655

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

eclipseisoffline
Copy link
Contributor

@eclipseisoffline eclipseisoffline commented May 11, 2024

This PR moves various settings from the NonVanillaCustomItemData to the CustomItemData class, which is useful for datapack/server side mod developers when porting custom items to Bedrock using Geyser's mapping system.

Properties that have been moved to the super-interface:

  • stackSize
  • maxDamage
  • attackDamage
  • toolType
  • toolTier
  • armorType
  • protectionValue
  • isHat
  • isFoil
  • isEdible
  • canAlwaysEat
  • isTool - deprecated, won't be moved

The builders for these classes have been correctly updated, alongside the implementations of these classes and their use in CustomItemRegistryPopulator.

A few notes:

  • The NonVanillaCustomItemData.Builder class has been given overrides for the moved properties to allow for full backwards compatibility.
  • The private isTool boolean field in the implementation of NonVanillaCustomItemData (GeyserNonVanillaCustomItemData) has been removed and not re-implemented in its super class - it has been replaced with displayHandheld and wasn't used anywhere in the implementation anymore. It was not publicly available. The isTool method has been moved to the super-interface and redirects to the displayHandheld method (it is marked deprecated, like before).
  • The constructor of GeyserCustomItemData has been simplified to simply take its builder as argument.
  • To not change the behaviour when the stack size is unset when extending vanilla items (in that case the Java base item's stack size is used), the stack size property has been changed slightly in implementation, which shouldn't be noticeable for API users. The Javadoc has been updated correctly:
    • Returns 0 if not set. When not set (or 0), takes the Java item stack count when based of a vanilla item, or uses 64 when porting a modded item.

  • To not change the behaviour when the max damage is unset when extending vanilla items (in that case the Java base item's max damage is used), the max damage property has been changed slightly in implementation, which shouldn't be noticeable for API users. The Javadoc has been updated correctly:
    • Returns -1 if not set. When not set (or below 0), takes the Java item max damage when based of a vanilla item, or uses 0 when porting a modded item.

  • The createComponentNbt method for CustomItemData has been updated to continue supporting GeyserMappingItem as well.
  • All properties moved are now supported in JSON item mappings as well, the MappingsReader_v1 has been updated accordingly.
  • The wiki has to be updated to reflect the changes (I could make the PR).
  • Unrelated to moving the properties, the implementation of the edible property has been updated to include the minecraft:use_modifiers Bedrock component alongside the minecraft:food component, which seems to now be required to make edible items behave correctly.

A couple of the new JSON mapping properties have been tested (stack_size, max_damage, hat and edible), and backwards compatibility of using builder methods in NonVanillaCustomItemData.Builder in extensions has been tested as well, which all seem to work okay.

Feel free to suggest any changes and I'll be sure to implement them!

@Camotoy
Copy link
Member

Camotoy commented May 12, 2024

Code looks fine to me, but will delegate to someone else for API approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants