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

Added Command registration Support! #31

Closed
wants to merge 9 commits into from

Conversation

thomas15v
Copy link
Contributor

No description provided.

@lenis0012
Copy link

I dont believe that Sponge likes to put 'I' in front of its classes.
And it is also missing some javadocs and the license

@viveleroi
Copy link
Contributor

Zidane mentioned he was planning a more "meaty" command system and I think with sk's recent work on Intake, and sk's desire for a non-crappy permission system, I'm betting that they'll want to tackle this.

@thomas15v
Copy link
Contributor Author

Well you can still go anywhere with this approach. Also I still think permissions should be handled on execution level of the command and not somewhere else.

@@ -1,4 +1,4 @@
package org.spongepowered.example;
package org.spongepowered.api;

Choose a reason for hiding this comment

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

Why change the package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh dangit I think Idea changed it when i tested the class.

@IDragonfire
Copy link

duplication of #29

Squashed commits:

[9187893] Removed my name from these Classes. (+1 squashed commits)

Squashed commits:

[ca7469a] Fixed Sponge denying field starting with a capital (+1 squashed commits)

Squashed commits:

[73083e5] Fixed license and sponge not liking "I" interfaces. (+1 squashed commits)

Squashed commits:

[7f607bf] Added Command registration Support!
@spaceemotion
Copy link

Pretty sure @sk89q is working on something like this based on his work on sk89q/Intake.

@IDragonfire
Copy link

@@ -10,10 +10,12 @@
@SpongeEventHandler
public void onInitialization(SpongeInitializationEvent event) {
event.game.getLogger().info("Hey folks, this is INITIALIZATION!");

Choose a reason for hiding this comment

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

Fix this

* @return
*/

public String getCommandUsage(CommandSender sender);

Choose a reason for hiding this comment

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

Do not use public in interfaces

@thomas15v
Copy link
Contributor Author

Oke working on it ;-;. (And ya i was looking at the wrong PR)

@turt2live
Copy link

@thomas15v Your PR does not merge. Please resolve the issue to have it reviewed further. Thanks!

@turt2live
Copy link

@thomas15v Also please identify how this PR is better/worse than #29 and why it should be pulled. Thanks!

Conflicts:
	src/main/example/main/java/org/spongepowered/example/ExamplePlugin.java
	src/main/java/org/spongepowered/api/Game.java
@thomas15v
Copy link
Contributor Author

Wait wait working on it. this isn't good ik.

Dangit it is broke somehow ;-;. Rly ik this would turn out bad.

@thomas15v
Copy link
Contributor Author

Finally! And @turt2live I honestly have no idea. This system should be a basic command system. Also the implementation is also working.

public void onServerStarting(SpongeServerStartingEvent event) {
event.game.getLogger().info("Hey...my implementation's server is starting?");
event.game.getCommandManager().registerCommand(new HelloWorldCommand());

Choose a reason for hiding this comment

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

umm, missing brace? "}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea how I lost that one.

@sk89q
Copy link
Contributor

sk89q commented Sep 16, 2014

I ended up writing the command interfaces myself, sorry!

See 8779e58.

@sk89q sk89q closed this Sep 16, 2014
@thomas15v
Copy link
Contributor Author

No Problem 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants