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

Sending components of an item in HoverEvent is not possible #3688

Open
3 of 4 tasks
Brokkonaut opened this issue Jun 1, 2024 · 6 comments
Open
3 of 4 tasks

Sending components of an item in HoverEvent is not possible #3688

Brokkonaut opened this issue Jun 1, 2024 · 6 comments

Comments

@Brokkonaut
Copy link
Contributor

Bungeecord version

BungeeCord-Bootstrap:1.20-R0.3-SNAPSHOT:52ab21b:1844

Server version

No response

Client version

1.20.6

Bungeecord plugins

n/a

The bug

When creating a HoverEvent with a Item content the components of that item are not shown on the client (this also applies to spigot) I tried this on spigot with

            String itemTag = stack.getItemMeta().getAsString();
            msg.setHoverEvent(new HoverEvent(Action.SHOW_ITEM, new Item(stack.getType().getKey().toString(), 1, itemTag != null ? ItemTag.ofNbt(itemTag) : null)));

(i also tried stack.getItemMeta().getAsComponentString()) - both don't work, only the item type is shown but no enchantments, custom name or whatever. It was working in 1.20.4.

Log output (links)

No response

Checking

  • I am using BungeeCord and not a fork. Issues with forks should not be reported here.
  • I think this is not an issue with a bungeecord plugin.
  • I have not read these checkboxes and therefore I just ticked them all.
  • This is not a question or plugin creation help request.
@md-5
Copy link
Member

md-5 commented Jun 1, 2024

cc/ @2008Choco

@2008Choco
Copy link
Contributor

2008Choco commented Jun 1, 2024

I'm unsure how you want to approach this now, but ItemSerializer tries to serialize it as id and tag, which doesn't exist anymore in vanilla. Id is now id (though I think we already serialize this), Count is now count, and tag is now components and is a Map of all the serialized components. So getAsComponentString() is correct, we're just not serializing it correctly in BungeeChat anymore. Do we want to have backwards compatibility for this?

An easily reproducible command is the followng:

/tellraw @p {"text":"[Hover me!]","hoverEvent":{"action":"show_item","contents":{"id": "minecraft:diamond_sword","count":1,"components":{"minecraft:enchantments":{"minecraft:sharpness":1}}}}}

EDIT:
Because Mojang isn't consistent for whatever reason, getComponentsAsString() returns the components surrounded in [] brackets as that's the format that's expected by /give. But that of course isn't valid when you're passing it to components in a hover event because it needs to be an actual map object, not a string. This suddenly got way more annoying to support because ItemTag now can't be represented as a string, but has to be represented as an object capable of being serialized identically to how components are serialized. We would need a BungeeCord equivalent of ItemMeta...

We could probably solve this in a really ugly way for Bukkit's own personal use by introducing a HoverEventProvider interface and having ItemStack implement it, but that doesn't solve the issue of supporting that in BungeeCord. It's also probably still annoying to implement in Bukkit. CraftBukkit would be fine, Bukkit, not so much. Immediate hacky solution I can think of at least is to serialize everything down and then manually concatenate the serialized string into the serialized JSON where "components" should be...

@Janmm14
Copy link
Contributor

Janmm14 commented Jun 1, 2024

As it is a problem in (de-)serialization i have these suggestions:

  • for improved drop-in compatibility we could assume incoming version = outgoing version and save a boolean/version integer in the Item / ItemTag. Bukkit can maybe set the default version to use
  • we need to introduce a second gson instance in ComponentSerializer and add a target minecraft version parameter

@md-5
Copy link
Member

md-5 commented Jul 10, 2024

We could probably solve this in a really ugly way for Bukkit's own personal use by introducing a HoverEventProvider interface and having ItemStack implement it, but that doesn't solve the issue of supporting that in BungeeCord. It's also probably still annoying to implement in Bukkit. CraftBukkit would be fine, Bukkit, not so much. Immediate hacky solution I can think of at least is to serialize everything down and then manually concatenate the serialized string into the serialized JSON where "components" should be...

I think this is fine, you put the interface in BungeeCord and have it return Map<String, Object> getAsComponent() or something to that effect (maybe even JsonElement since it can be specific to BungeeCord). If a Bungee plugin wants to display an Item of its own volition, then it can just parse that item from JSON (like it already would need to do for ItemTag/NBT)).

@2008Choco
Copy link
Contributor

2008Choco commented Jul 10, 2024

and have it return Map<String, Object> getAsComponent() or something to that effect (maybe even JsonElement since it can be specific to BungeeCord)

I think maybe instead we have it return a HoverEvent, no? Something like this:

public interface HoverEventProvider {

    public HoverEvent getAsHoverEvent(); // or just asHoverEvent()

}

@md-5
Copy link
Member

md-5 commented Jul 10, 2024

Sorry yeah that probably makes more sense

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

No branches or pull requests

4 participants