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

Add Translatable interface for fluid creation of TranslatableComponents #3558

Closed
wants to merge 3 commits into from

Conversation

2008Choco
Copy link
Contributor

@2008Choco 2008Choco commented Nov 3, 2023

This PR adds a simple TranslationProvider interface to more fluidly create TranslatableComponent instances. While nothing in the Bungee API needs to implement TranslationProvider, the Spigot API can most certainly use this interface for types such as ItemType, BlockType, EntityType, Enchantment, so on and so forth.

While yes Bukkit does already have a Translatable interface, it cannot be used to directly construct a TranslatableComponent which makes working with it cumbersome and forcing developers to access the translation key rather than passing the translatable object directly. Bukkit's Translatable interface may extend BungeeChat's TranslationProvider interface for convenience.

// Before
TranslatableComponent component = new TranslatableComponent(Material.STONE.getTranslationKey());
// or
BaseComponent component = new ComponentBuilder()
    .append(new TranslatableComponent(Material.STONE.getTranslationKey()))
    .build();

// After
TranslatableComponent component = new TranslatableComponent(Material.STONE);
// or
BaseComponent component = new ComponentBuilder()
    .append(Material.STONE)
    .append(Material.STONE.asTranslatableComponent()) // Alternatively
    .append(Material.STONE.asTranslatableComponent("first arg", 2)) // Can be used to pass args as well
    .build();

Perhaps ComponentBuilder#append(TranslationProvider) is not clear enough, in which case I propose appendTranslatable() instead as a clearer distinction. I'm open to either one.

@Janmm14
Copy link
Contributor

Janmm14 commented Nov 3, 2023

To not confuse users and due to the intended usage inside spigot, the Translateable interface in bungee should have a different name from the existing spigot one.

@2008Choco
Copy link
Contributor Author

2008Choco commented Nov 3, 2023

To not confuse users and due to the intended usage inside spigot, the Translateable interface in bungee should have a different name from the existing spigot one.

I disagree. I think there are plenty of examples of instances where BungeeChat has clashing names with Bukkit classes. ChatColor and some hover contents (Entity and Item) come to mind. The most common use case of this interface would be to pass instances into a TranslatableComponent or a ComponentBuilder. It would be rare for developers to specifically want a Translatable instance as a parameter, and if they do, I'm certain they should be competent enough to understand how to correctly declare packages to distinguish the two types.

If @md-5 agrees with you however, I would at least like to propose one of the following alternatives:

  • TranslatableObject
  • TranslationSource
  • TranslationProvider

@Janmm14
Copy link
Contributor

Janmm14 commented Nov 3, 2023

Item doesnt clash, ChatColor is due to pre-component-api

@md-5
Copy link
Member

md-5 commented Nov 3, 2023

The Bungee clash causes big issues, but also ChatColor is used a lot more.

That being said, I probably like TranslationProvider the best

@2008Choco
Copy link
Contributor Author

I have no issues with TranslationProvider (it was my preferred option as well). Updated and ready to go 👍

@md-5 md-5 self-requested a review November 7, 2023 09:19
md-5 pushed a commit that referenced this pull request Nov 9, 2023
md-5 pushed a commit that referenced this pull request Nov 9, 2023
@md-5 md-5 closed this Nov 9, 2023
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.

3 participants