Adds weapon, break_sound, equippable, and swing_animation item components. - #2821
Adds weapon, break_sound, equippable, and swing_animation item components.#2821heypr wants to merge 9 commits into
weapon, break_sound, equippable, and swing_animation item components.#2821Conversation
| } | ||
| if (value.allowedEntities() != null) { | ||
| ListTag entities = new ListTag(); | ||
| value.allowedEntities().forEach(key -> entities.addObject(new ElementTag(key.key().asMinimalString(), true))); |
There was a problem hiding this comment.
Meta says ListTag(EntityTag) (which is probably correct for this), but these are ElementTags
Also, should be able to use RegistryKeySet#values with the LisTag convertor constructor instead of looping over
|
|
||
| @Override | ||
| public Equippable fromDenizen(MapTag value, Mechanism mechanism) { | ||
| ElementTag slot = value.getObjectAs("slot", ElementTag.class, mechanism.context); |
There was a problem hiding this comment.
I think this can just be #getElement?
| setIfValid(builder::shearSound, value, "shear_sound", ElementTag.class, null, element -> Utilities.parseNamespacedKey(element.asString()), "namespaced key", mechanism); | ||
| ListTag entityList = value.getObjectAs("allowed_entities", ListTag.class, mechanism.context); | ||
| if (entityList != null) { | ||
| List<TypedKey<EntityType>> keys = new ArrayList<>(); |
There was a problem hiding this comment.
Nitpick, but can initialize to the correct size here
| // @description | ||
| // Controls an item's swing animation <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "animation_type", an ElementTag representing the animation type. Valid animation types can be found at <@link url https://jd.papermc.io/paper/io/papermc/paper/datacomponent/item/SwingAnimation.Animation.html> |
| // Controls an item's swing animation <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "animation_type", an ElementTag representing the animation type. Valid animation types can be found at <@link url https://jd.papermc.io/paper/io/papermc/paper/datacomponent/item/SwingAnimation.Animation.html> | ||
| // - "duration", an ElementTag(Number) representing the duration of the animation. |
| public SwingAnimation fromDenizen(MapTag value, Mechanism mechanism) { | ||
| SwingAnimation.Builder builder = SwingAnimation.swingAnimation(); | ||
| setIfValid(builder::type, value, "animation_type", ElementTag.class, | ||
| element -> element.matchesEnum(SwingAnimation.Animation.class), |
There was a problem hiding this comment.
Can test, but pretty sure this is redundant - asEnum would return null for invalid input and setIfValid will treat that as invalid input the same way
There was a problem hiding this comment.
What would be the appropriate way to handle it?
| // @description | ||
| // Controls an item's weapon <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "disable_blocking_duration", an ElementTag(Decimal) representing the number of seconds that a shield will be disabled for after blocking an attack from this item. |
| // Controls an item's weapon <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "disable_blocking_duration", an ElementTag(Decimal) representing the number of seconds that a shield will be disabled for after blocking an attack from this item. | ||
| // - "item_damage_per_attack", an ElementTag(Number) representing the amount of durability damage this item will take when used to attack an entity or break a block. |
There was a problem hiding this comment.
durability_per_attack maybe? Just to match the naming of existing features
| // @description | ||
| // Controls an item's swing animation <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "animation_type", an ElementTag representing the animation type. Valid animation types can be found at <@link url https://jd.papermc.io/paper/io/papermc/paper/datacomponent/item/SwingAnimation.Animation.html> |
There was a problem hiding this comment.
Also just realized this makes the key swing_animation.animation_type - the double animation is redundant there imo
| map.putObject("camera_overlay", new ElementTag(value.cameraOverlay().asMinimalString(), true)); | ||
| } | ||
| if (value.allowedEntities() != null) { | ||
| ListTag allowedEntities = new ListTag(value.allowedEntities().values(), key -> new EntityTag(DenizenEntityType.getByName(key.key().value()))); |
There was a problem hiding this comment.
Just use the Registry API to get the type from the key directly - Registry.ENTITY_TYPE.getOrThrow(key)
| MapTag map = new MapTag(); | ||
| map.putObject("slot", new ElementTag(value.slot())); | ||
| map.putObject("equip_sound", new ElementTag(value.equipSound().asMinimalString(), true)); | ||
| if (value.assetId() != null) { |
There was a problem hiding this comment.
These keys not always being in tag output isn't documented (can probably go in a @tag section?)
| // - "damage_on_hurt", a ElementTag(Boolean) controlling whether the item takes damage when the wearer is hurt. | ||
| // - "equip_on_interact", a ElementTag(Boolean) controlling whether the item is equipped on entity interaction. | ||
| // - "can_be_sheared", a ElementTag(Boolean) controlling whether the item can be sheared off an entity. | ||
| // - "shear_sound", an ElementTag representing the sound played when shearing using this item in namespaced key format. |
There was a problem hiding this comment.
I don't think this is true? iirc it's the sound played when shearing this item off of an entity that's wearing it
| // - "camera_overlay", an ElementTag representing the camera overlay to use when the item is equipped in namespaced key format. | ||
| // - "allowed_entities", a ListTag(EntityTag) representing entity types that can equip this item. If not set, all entities are allowed to wear this item. | ||
| // - "dispensable", a ElementTag(Boolean) controlling whether the item can be dispensed. | ||
| // - "swappable", a ElementTag(Boolean) controlling whether the item can be swapped. |
There was a problem hiding this comment.
Needs to be more clear about referring to the right click armor swapping feature
| // @input MapTag | ||
| // @description | ||
| // Controls the properties of an item's equippable <@link language Item Components>. | ||
| // The map includes keys: |
There was a problem hiding this comment.
This needs to make it clear which keys are optional (and what their default is) - the meta sounds like you might need every single key when actually all you need is slot
| for (String entry : entityList) { | ||
| keys.add(TypedKey.create(RegistryKey.ENTITY_TYPE, Utilities.parseNamespacedKey(entry))); | ||
| } |
There was a problem hiding this comment.
Was this tested with actual EntityTag objects in the list? I believe that would break because of the e@ in the strings
This should loop over the list as EntityTags (ListTag#filter), get the entity type, and then create the TypedKey from EntityType#key.
| // Controls an item's weapon <@link language Item Components>. | ||
| // The map includes keys: | ||
| // - "disable_blocking_duration", a DurationTag representing the duration that a shield will be disabled for after blocking an attack from this item. | ||
| // - "durability_per_attack", an ElementTag(Number) representing the amount of durability damage this item will take when used to attack an entity or break a block. |
There was a problem hiding this comment.
Are you sure about the or break a block? I believe that's controlled by the tool component
As stated in the title, this PR adds four item components :)