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 NamespacedKey resolver #24

Closed
wants to merge 3 commits into from
Closed

add NamespacedKey resolver #24

wants to merge 3 commits into from

Conversation

Grabsky
Copy link
Contributor

@Grabsky Grabsky commented May 31, 2022

this PR adds value resolver for org.bukkit.NamespacedKey with resource_location brigadier type to support nice client-side formatting

bumped Lamp version to 3.0.4-SNAPSHOT

…ture/more-arguments

# Conflicts:
#	bukkit/src/main/java/revxrsal/commands/bukkit/exception/BukkitExceptionAdapter.java
#	bukkit/src/main/java/revxrsal/commands/bukkit/exception/InvalidNamespacedKeyException.java
@Grabsky
Copy link
Contributor Author

Grabsky commented May 31, 2022

added missing final keywords to parameters

@Revxrsal
Copy link
Owner

This PR looks good to me, though I wonder if it would be better to merge that, or add a way to register your own Brigadier arguments and then you can add as many fancy brigadier stuff as you want?

@Grabsky
Copy link
Contributor Author

Grabsky commented May 31, 2022

Yes, that'd be much better and more flexible. I thought about that already and by taking inspiration from cloud, I think there could be something like BrigadierManager which instance could be accessed from BukkitCommandHandler.

final BrigadierManager brigadier = BukkitCommandHandler#getBrigadierManager();

And I think there should be two methods in that class:

// Class<?>, ArgumentType<?>  -  for instances of brigadier's ArgumentType
brigadier.bind(NamespacedKey.class, ResourceLocationArgument.id());
// Class<?>, String  -  for 'minecraft:' keys specifically; using MinecraftArgumentType.getByKey(String)
brigadier.bind(NamespacedKey.class, "resource_location"));

Second method could technically fallback to the first once it gets non-null ArgumentType<?> from Minecraft internals.

BrigadierManager class could later provide more brigadier-exclusive API (supposing it has more features, not entirely sure if there is anything worth noting).

Naming is just an example; let me know what you think about such structure.

@Revxrsal
Copy link
Owner

Revxrsal commented Jun 8, 2022

Brigadier API has been implemented. You can freely implement this feature yourself

@Revxrsal Revxrsal closed this Jun 8, 2022
@Grabsky Grabsky deleted the feature/more-arguments branch June 8, 2022 20:50
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.

None yet

2 participants