Skip to content

add item_name property#2838

Open
creaturesofhabit wants to merge 5 commits into
DenizenScript:devfrom
creaturesofhabit:item-component
Open

add item_name property#2838
creaturesofhabit wants to merge 5 commits into
DenizenScript:devfrom
creaturesofhabit:item-component

Conversation

@creaturesofhabit

Copy link
Copy Markdown

For https://discord.com/channels/315163488085475337/1362329776475603057. Works as expected on 26.2 and 26.1.

Considered mentioning this property in the item.display documentation since this is probably to be preferred for most use-cases, but didn't want to mess with other files.

@tal5 tal5 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also feel like the meta could explain how this works in relation to ItemTag.display better? But I don't really have any rewrite suggestions so it could be fine as-is for now


@Override
public ElementTag toDenizen(Component value) {
return new ElementTag(LegacyComponentSerializer.legacyAmpersand().serialize(value));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These would lose all advanced text formatting, you want the methods from PaperModule here (and in the other method)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed that. Is there a way I should have known that those methods existed other than just browsing a bunch of code in the module?

@Hydroxycobalamin

Copy link
Copy Markdown
Contributor

I was looking into PRing this too.

To make meta clearer I would suggest something like:

Controls the item's item_name component, which cannot be altered by a player. It also sets the item's default display name, which is modifiable. Even if the display name is changed, for example by renaming the item in an anvil, item_name retains its original value.

Mentioning it on the ItemTag.display mechanism is a good idea from my perspective, as it might be confusing for people.

I would add:

To set the default display name of an item use: <link>

pending change to itemtag.display meta
DataComponentAdapter.register(new MaxDurabilityAdapter());
DataComponentAdapter.register(new MaxStackSizeAdapter());
DataComponentAdapter.register(new RarityAdapter());
DataComponentAdapter.register(new ItemNameAdapter());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical

// @name item_name
// @input ElementTag
// @description
// Controls the item's item_name component. In effect, this changes its display name. Unlike <@link mechanism ItemTag.display>, this name cannot be changed by an anvil, but will be hidden when the display is set, such as with an anvil. That is, it will remain under the display name and be shown again if the display is removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is really long, you can have multiple lines divided by sentence when necessary.

@creaturesofhabit

Copy link
Copy Markdown
Author

Should item script containers use this instead of or in addition to display? I think it's generally a better option for 98% of use cases. Having a display name and item name key would be extremely confusing, but of course just changing it out would be breaking even if it weren't paper-only

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.

4 participants