Skip to content

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.

# Conflicts:
#	Essentials/src/main/java/com/earth2me/essentials/commands/Commandessentials.java
@JRoy JRoy mentioned this pull request Jul 22, 2025
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