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

Simplify and fix some issues in /script command registration #1751

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

altrisi
Copy link
Collaborator

@altrisi altrisi commented Jun 26, 2023

This PR refactors the /script command registration a bit, mostly to make it more clear when adding to brigadier and to fix some issues with it.

Summary of changes:

  • /script in <app> will no longer silently use the default host when app isn't valid, but instead will fail
  • Global /script actions such as stop or resume will no longer be repeated under each /script in <app>
  • Suggestion initialization will no longer sort on insert but at the end, and will then use unmodifiable collections
  • Hides /script download if app store is completely disabled
  • Simplifies permissions: Now the whole /script is under the same rule
  • No longer stores all individual subcommands in variables with meaningless names, but uses the typical fluent API of brigadier
  • Removes unnecessary Stream->List conversions for some suggestions
  • Makes all methods in multiline chains consistently have the dot at the start of the line instead of at the end of the previous one to prevent confusion with direct method calls

There may be more changes I've missed given most of this has been in a git stash for a while, though I think I got them all.

@altrisi altrisi added the scarpet Issues related to scarpet in some way label Jun 26, 2023
@altrisi altrisi requested a review from gnembon June 26, 2023 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scarpet Issues related to scarpet in some way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant