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.

Sign up for GitHub

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

Jump to bottom

Implement new high-level command API #1400

Closed
wants to merge 1 commit into from
Closed

Implement new high-level command API #1400

wants to merge 1 commit into from

Conversation

dualspiral
Copy link
Contributor

@dualspiral dualspiral commented Jun 18, 2017 •
edited

Sponge API | SpongeCommon | SpongeForge

This now works - see API PR for more description.

@dualspiral dualspiral self-assigned this Jun 18, 2017
@dualspiral dualspiral mentioned this pull request Jun 18, 2017
Closed
@kashike kashike self-requested a review June 19, 2017 23:41
@michaelzangl
Copy link

Just if you need it: For Minebot I needed a lot of commands. This is why I wrote my own command parser. Parsing is done by the CommandDefinition class. Commands can simply be defined in a class:

@AICommand(name = "minebuild", helpText = "Fill an area with a given block.")
public class CommandFillArea {
	public static AIStrategy run(
		AIHelper helper,
		@AICommandParameter(type = ParameterType.FIXED, fixedName = "fill", description = "") String nameArg,
		@AICommandParameter(type = ParameterType.BLOCK_NAME, blockFilter=FillAreaPathfinder.FillBlocks.class, description = "The block to place") BlockWithDataOrDontcare block) {
		...
	}
}

This automatically adds the help text for the command with the parameter description. Tab complete is fully supported. Parameter types can be blocks, enums, strings, numbers, BlockPos. It is could be made easy to add a new parameter type by using a class instead of an enum constant.
Command invocations are then:
/minebuild fill stone

(currently licensed under GPL, but I have no problem with re-licensing my parts under MIT)

@dualspiral dualspiral force-pushed the refactor/commands-to-impl-1 branch from 7b499a9 to e1de803 Compare July 3, 2017 22:10
@gabizou
Copy link
Member

gabizou commented Jul 9, 2017

If you can, when you update the implementation with bleeding, provide a test plugin to make some examples of how things would look with the API changes. I think it would suit better with the description overall.

@dualspiral
Copy link
Contributor Author

Okie doke, will do.

@dualspiral dualspiral requested a review from gabizou as a code owner July 9, 2017 11:31
@dualspiral dualspiral mentioned this pull request Jul 22, 2017
Closed
@ryantheleach
Copy link
Contributor

This enables #1463 right?

@dualspiral
Copy link
Contributor Author

Yes.

@parlough
Copy link
Contributor

@dualspiral Making all of our dreams come true.

@zml2008
Copy link
Member

zml2008 commented Jul 25, 2017

it's finally happening!

kashike
kashike suggested changes Jul 25, 2017
View reviewed changes
src/main/java/co/aikar/timings/TimingsExport.java Outdated
@@ -116,7 +116,7 @@ static void reportTimings(CommandSource sender) {
.add("maxmem", runtime.maxMemory())
.add("cpu", runtime.availableProcessors())
.add("runtime", ManagementFactory.getRuntimeMXBean().getUptime())
.add("flags", RUNTIME_FLAG_JOINER.join(runtimeBean.getInputArguments()))
.add("setFlags", RUNTIME_FLAG_JOINER.join(runtimeBean.getInputArguments()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no

kashike
kashike reviewed Jul 25, 2017
View reviewed changes
src/main/java/org/spongepowered/common/command/parameter/flag/SpongeFlags.java Outdated
}

private ArgumentParseException createValueError() {
return createError(t("Short setFlags that are not at the end of a group cannot have a value."));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be flags, not setFlags

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get this right one day...

@dualspiral dualspiral force-pushed the refactor/commands-to-impl-1 branch 2 times, most recently from 61062d5 to 4044530 Compare November 5, 2017 17:33
@ryantheleach ryantheleach added this to the API 8.0 milestone Nov 27, 2017
@dualspiral dualspiral force-pushed the refactor/commands-to-impl-1 branch 2 times, most recently from 4c0b09b to d49591a Compare December 3, 2017 09:46
@dualspiral dualspiral force-pushed the refactor/commands-to-impl-1 branch 2 times, most recently from 4e9e595 to 7b9980c Compare January 14, 2018 19:01
@dualspiral
Implement the new command API
aee097f 
* Make commands accept a cause
* Update test plugins
* Add parameters from SpongePowered/SpongeAPI#1605
* Add colour parameter
@stephan-gh stephan-gh removed their request for review August 10, 2018 17:43
@dualspiral dualspiral mentioned this pull request Feb 2, 2019
Closed
@dualspiral
Copy link
Contributor Author

Closed in favour of SpongePowered/SpongeAPI#1958 for 1.13

@dualspiral dualspiral closed this Feb 2, 2019
@dualspiral dualspiral deleted the refactor/commands-to-impl-1 branch June 3, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kashike kashike kashike requested changes

@gabizou gabizou Awaiting requested review from gabizou

@Deamon5550 Deamon5550 Awaiting requested review from Deamon5550

Assignees

@dualspiral dualspiral

Labels
status: wip Work in progress system: command
Projects
None yet
Milestone
API 8.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@dualspiral @michaelzangl @gabizou @ryantheleach @parlough @zml2008 @kashike