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

Introduce command trees; improve Commandessentials #5384

Draft
wants to merge 9 commits into
base: 2.x
Choose a base branch
from

Conversation

mdcfe
Copy link
Member

@mdcfe mdcfe commented Jun 7, 2023

Information

This PR closes an internal goal.

Details

Proposed feature:

This implements two separate features (may be split into separate PRs later down the line):

  • Introduce command trees, which allow for nested subcommands to be declared and parsed effectively.
    • Goals: lightweight, incrementally adoptable across new and existing commands as necessary
    • Trees consist of a root node, literal nodes and execute nodes
    • No parsing of argument types is implemented at present
    • Possibly worth exploring cloud/another tree command lib instead of rolling our own
    • Provide helper methods on command exec context to replicate EssentialsCommand
    • Think about command node impl in more depth
  • Rewrite /essentials, splitting complex subcommands into separate classes
    • Replace run and tabComplete impls with tree
    • Refactor existing subcommand functions
    • Split out dump into own class (possibly a separate refactor on its own?)

Environments tested:

OS: Windows 11 22H2

Java version: Eclipse Temurin 17.0.6

  • Most recent Paper version (1.19.4, git-Paper-547)
  • CraftBukkit/Spigot/Paper 1.12.2
  • CraftBukkit 1.8.8

Demonstration:

This PR should preserve all existing functionality of /essentials.

@YanisBft
Copy link
Contributor

YanisBft commented Jun 9, 2023

Dumb question maybe, but wouldn't it be possible to use Mojang's brigadier library for this purpose?

@JRoy
Copy link
Member

JRoy commented Jun 10, 2023

Dumb question maybe, but wouldn't it be possible to use Mojang's brigadier library for this purpose?

Using that severely limits the flexibility we have and reduces the code quality.

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