GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.88k stars 271 forks source link

GlowCommandWithSubcommands #1038

Closed Pr0methean closed 5 years ago

Pr0methean commented 5 years ago

Factors out some logic for implementing commands whose first argument is a subcommand. GlowstoneCommand, WeatherCommand and TimeCommand will be the first ones to benefit.

aramperes commented 5 years ago

Should we be doing this, considering we'll have to revamp the command system to support the Brigadier/1.13 command format in the near future anyway?

To me this seems like an unnecessary abstraction.

Pr0methean commented 5 years ago

If that argument is valid, then it's valid for all command i18n. Is 1.13 (or at least, a merge of the Brigadier part into dev) coming soon enough for that to be acceptable? If not, is there a branch with Brigadier I can PR the rest of the command i18n against?

aramperes commented 5 years ago

@Pr0methean I think our current implementation of i18n is fine, and we'll be able to use the 1.13 command structure with it. But I don't think abstracting the subcommand logic like this PR does is necessary.

mastercoms commented 5 years ago

Closed due to above comments about 1.13.