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

Namespace support for background texture #22

Open
JasperBouwman opened this issue Dec 28, 2024 · 2 comments
Open

Namespace support for background texture #22

JasperBouwman opened this issue Dec 28, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@JasperBouwman
Copy link

Hello,

I am looking at your API, and my first impression is that it looks nice and should work for my plugin.
As I am experimenting with some of the features, I wanted to set the background of a new tab I created. But in here I have an issue:

The default namespace you use is Minecraft, can you change the type of the background texture from String to NameKey, so that you can set your own namespace?
In PacketConverter.java at line 64 you will have to use the constructor of the Resource location.

For my plugin I use the namespace 'Minecraft' for the minecraft models, and in the 'tport' namespace folder I place my own custom textures. I do this to separate Minecraft and my plugin. TPort is the name of my plugin.

The next issue I have is that you can only have one BaseComponent inside your AdvancementDisplay.
As a final touch, for this display I would like to directly input this title/description as JSON, since for my plugin I've wrote my own message handler (not using the TextComponent API from spigot). Because of this I can not use your API directly.
This could be a quick fix. In your JSONMessage you can store an array with BaseMessage or a String with the raw JSON.
In the method of getBaseComponent you will check if one of these types are non null, and parse this in the ChatSerializer (with the BaseComponent using ComponentSerializer.toString).

I hope you will take this in consideration.

Best regards,
Jasper

@ZockerAxel
Copy link
Owner

Hey, thanks for your feedback.

Regarding background texture namespace

Internally, ResourceLocation#parse(string) is used to convert the background texture into a ResourceLocation (as you probably saw because you pointed out that that's where we would use the constructor instead of parse) that is sent to the client. As far as I know that means that it should support namespaces prefixed to the path like usual: "namespace:path".
Using a NameKey for background textures would IMO a better design decision so I will see if I can find a nice way of adding it without breaking backwards compatibility.

Regarding the BaseComponent count limitation

Again, this could be done but it is at the cost of breaking backwards compatibility. You can already have one "root" TextComponent and use TextComponent#addExtra and you have the same functionality, so unlikely for me to add unless there is a specific use-case.

Regarding direct JSON strings

This is something I could definitely do, I don't see any downsides for backwards compatibility and it sounds like a quality-of-life improvement that has some real use-cases.

@ZockerAxel ZockerAxel added the enhancement New feature or request label Dec 29, 2024
@ZockerAxel ZockerAxel self-assigned this Dec 29, 2024
@JasperBouwman
Copy link
Author

Hey ZockerAxel,
I just want to let you know that using 'namespace:path' does work. But using your NameKey would indeed still be better.

I'm looking forward for the next update that does support these enhancements

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

No branches or pull requests

2 participants