Make ItemStack#displayName only return the display name

[Phoenix616]

Previously additional formatting like brackets around the display name as well as the item information hover event were automatically added when using the ItemStack#displayName. In my opinion this did not match the API specification ("Get the formatted display name of the ItemStack", that says nothing about additional characters or hover information) and made it impossible to get the "raw" display name component (which in comparison to ItemMeta#displayName fell back to the default translation if no displayName was set) without any additional data. (Especially the brackets were hard to remove)

This changes how the ItemFactory#displayName method gets the display name: Instead of directly using the NMS ItemStack#getDisplayName method (which isn't really correctly named imo) it now directly uses the NMS ItemStack#getHoverName and applies the italic formatting with the same logic as Vanilla would. We end up with a component that only contains the name as the text without additional characters or hover which Vanilla uses for display purposes in chat e.g. on /give.

Old:

New:

The old (wrong) functionality can still easily be achieved with existing API by simply adding the hover event (ItemStack#asHoverEvent) to the displayName Component like item.displayName().hoverEvent(item) or if the brackets are desired Component.translatable("chat.square_brackets", item.displayName()).hoverEvent(item).

[jpenilla]

that says nothing about additional characters or hover information

This is a documentation issue, not a reason to change the behavior of a method that's been around for multiple major versions out of the blue.

The method is named correctly, and returning the displayName as used in vanilla commands is 100% the intended behavior (as noted in the PR adding the method, however again I agree the docs are too light), if anything the displayName methods on ItemMeta are the ones named wrong, they should be called customName.

I think a hoverName utility method could be useful... although it looks like we closed a similar PR before: #8793

[Phoenix616]

I do not agree with this just being a documentation issue. If anything this is an API design issue as the method is returning information that is clearly not a name: It's adding additional formatting not part of the name (the brackets around the name) as well as information unnecessary for the purpose the method appears to be for: To provide the displayed name of the ItemStack.

Also besides what is claimed on the linked PR this is not something that you can easily do in a different way as Component.translatable(itemStack) does only return a translatable Component based on the raw item translation key without additional information (player heads should include the owner name and books the title. And they do so in NMS ItemStack#getDisplayName/getHoverName) and ItemMeta#displayName will (of course) only return the renamed display name of the item.

Also in general: I don't think that the API should follow what Mojang calls internal methods/properties as here a usable and user-friendly API should be designed (and imo that includes using obvious and non-confusing names), something that Mojang doesn't need to think about. So just because Mojang calls it a "display name" and the other thing a "hover name" (something that doesn't even make sense imo. and shouldn't be exposed in the API under that name at all) it doesn't mean we need to use their wording if it doesn't fit in the overall plugin API.

I realise that changing behaviour sucks and that this should've been caught back when it was first implemented but it wasn't and now we are here. And if the goal is to not break backwards compatibility here then of course ItemStack#displayName could be deprecated (as it could be considered confusing anways as people think it's ItemMeta#displayName) and two different methods could be added with better names but I personally doubt that is necessary here as a) it's a visual-only change and b) a backwards-compatible workaround exists as shown in the last part of the original PR comment.

A method providing the current implementation could be called something like itemInformation, itemDisplay, dataDisplay, informationDisplay or itemDataComponent but not just a "name" when it returns more information than just the name. Alternatives for the just-name method could be itemName, displayedName or just name to avoid the confusion with ItemMeta#displayName.

[electronicboy]

deprecating displayName would be preferrable to blindly changing the behavior which has been set forth randomly during the middle of a patch release

[Owen1212055]

Generally, the name is kinda meh, but I agree that this is rather weird behavior.

[Phoenix616]

@jpenilla can you take a look at this again (or someone else say what the state is here? 👀)

[Phoenix616]

Is there any chance for something like this getting pulled? Or is this feature just not getting accepted? Don't really want to waste time updating this to 1.20 if the whole thing isn't wanted...

[zml2008]

The original logic we had when talking about this was to create a distinction between a "display name" (the item name formatted for use in a command output like Vanilla has) and a "custom name" which is just the Component that is the custom name.

What you seem to be hoping for is a third thing, the custom name if present or otherwise the translation key (Item#getName(stack)) -- this is what vanilla calls the "hover name", imo even a worse name than "display name".

Does paper api expose all 3 of these? imo the best way forward is to add the missing concepts to the API without renaming/changing behavior on anything that currently exists. The end naming might not be ideal, but hopefully JD can help users navigate their options.

[lynxplay]

I think the best way forward would be to express the logic you are after (which I 100% agree, this is functionality used so often, API sugar is useful) in a new method, maybe named ItemStack#name and heavily expand the javadocs on the poorly named displayName method.

If you are down for that, I'd be happy if you could update the PR, breaking the API contract now is pretty out of the question and I don't think we are interested in the PR.

[Phoenix616]

Ok, I will look into getting this updated and the suggestions incorporated soon™️

[Owen1212055]

When discussing, I am not sure how much of a fan we are of displayedName, I still perhaps think name() is better in this case? In general, the naming is just confusing.

Data display is also a bit odd, as this really only occurs in chat, so I think something like chatDisplay might help describe this better.

Donno, further opinions wanted.

[Elikill58]

I support this PR