Fixed command system issues #119

Tais993 · 2021-09-15T09:23:48Z

Moved AbstractCommand to the Commands package instead of the example package, this was an accident.
Improved docs:

Added docs to the AbstractCommand constructor
Added multiple @see tags
Elaborated on the effect of Command#isGuildOnly

And optimized imports

application/src/main/java/org/togetherjava/tjbot/commands/AbstractCommand.java

application/src/main/java/org/togetherjava/tjbot/commands/Command.java

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java

application/src/main/java/org/togetherjava/tjbot/commands/example/CommandExample.java

application/src/main/java/org/togetherjava/tjbot/commands/example/SubCommandExample.java

Tais993 · 2021-09-16T09:07:34Z

Made a few mistakes causing the merge commit, I'll squash

application/src/main/java/org/togetherjava/tjbot/db/package-info.java

I-Al-Istannen

I'd prefer qualifying the javadoc link and not the code itself.

application/src/main/java/org/togetherjava/tjbot/commands/example/CommandExample.java

Tais993 · 2021-09-18T19:15:25Z

Just a suggestion, but maybe you should learn Java @Tais993

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java

d3vel0per · 2021-09-18T20:44:11Z

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java

+        new ErrorHandler().handle(ErrorResponse.MISSING_ACCESS, errorResponseException -> {
+            // * Hard to read, there's no other accurate way to get a channel you've the write
+            // * permission in.
+            guild.getTextChannelCache().stream().filter(textChannel -> {


This can be a utility method Optional<TextChannel> getTextChannelWithWritePermissions(Guild) to aid readability

Creating something like

ChannelUtil#getChannelsWithPermissions(Guild, Permissions) seems fair
But this should be done within another PR

Ok. I have already created one JDAUtils. I will move it in my PR instead.

Don't remember JDAUtils having anything relating, and JDAUtils is extremely outdated

🤦 thats something I have written and will pull out your piece.

Ah, thought you were talking about JDA utlities

d3vel0per · 2021-09-18T20:44:54Z

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java

+            }).findFirst().ifPresent(textChannel -> {
+                textChannel
+                    .sendMessage("This bot needs the commands scope, please invite it correctly!\n"
+                            + "You can join https://discord.gg/UJBwS2Hvcx for more info, I'll leave now.")


What is this url? May be a constant

Together Java Discord, should be a constant in the main/application/whatever the main is called in that case imo

Should also be moved out into the "config system" #100

See PR #142 which will add a discordGuildInvite property for this.

d3vel0per · 2021-09-18T20:45:41Z

application/src/main/java/org/togetherjava/tjbot/commands/example/CommandExample.java

@@ -47,17 +49,20 @@ public CommandExample() {
                // * adds the second option
                new OptionData(OptionType.STRING, "times-to-ping",
                        "Amount of times the user will be pinged, default 1")
-                            .addChoices(new Command.Choice("Once", "1"),
-                                    new Command.Choice("Twice", "2"))
+                            .addChoices(


Not sure if this is formatter related but the deleted lines look more readable to me.

I'd assume formatter since I don't remember it myself

marko-radosavljevic

spotlessApply ^^

Tais993 · 2021-09-20T07:47:30Z

Since requested, requires some time to develop a proper permission system.

Zabuzard · 2021-09-27T07:29:18Z

Since requested, requires some time to develop a proper permission system.

This branch contains quite some useful improvements to the command system. If the permission thing needs more time, why not split this into two PRs and get this finally merged?

Tais993 · 2021-09-28T11:11:42Z

Sure, will look at this later today.

Zabuzard · 2021-10-08T07:23:31Z

Sorry, I messed up the rebase. Ill rescue it, gimme a minute

Zabuzard · 2021-10-08T08:15:36Z

Split this into #169 and #170. Cheers

Zabuzard requested changes Sep 15, 2021

View reviewed changes

Zabuzard assigned Tais993 Sep 15, 2021

Zabuzard added the enhancement New feature or request label Sep 15, 2021

Zabuzard added this to the Initial Setup milestone Sep 15, 2021

Tais993 mentioned this pull request Sep 16, 2021

Migrate basic commands (ping, dbget, dbput) #127

Merged

Tais993 requested a review from Zabuzard September 16, 2021 09:07

marko-radosavljevic previously approved these changes Sep 16, 2021

View reviewed changes

Zabuzard requested changes Sep 16, 2021

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/db/package-info.java Outdated Show resolved Hide resolved

Tais993 dismissed marko-radosavljevic’s stale review via 7382386 September 16, 2021 19:33

Tais993 force-pushed the fix-issues-command-system branch from 857ab95 to 7382386 Compare September 16, 2021 19:33

marko-radosavljevic self-requested a review September 16, 2021 19:36

marko-radosavljevic previously approved these changes Sep 16, 2021

View reviewed changes

Tais993 requested a review from Zabuzard September 17, 2021 07:29

Tais993 dismissed marko-radosavljevic’s stale review via 4596044 September 17, 2021 13:48

Tais993 requested review from a team as code owners September 17, 2021 13:48

I-Al-Istannen previously approved these changes Sep 18, 2021

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/example/CommandExample.java Outdated Show resolved Hide resolved

Tais993 commented Sep 18, 2021

View reviewed changes

Tais993 dismissed I-Al-Istannen’s stale review via ddd5a1c September 18, 2021 19:12

Tais993 requested a review from I-Al-Istannen September 18, 2021 19:15

marko-radosavljevic previously approved these changes Sep 18, 2021

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java Outdated Show resolved Hide resolved

Tais993 dismissed marko-radosavljevic’s stale review via c8d93c6 September 18, 2021 19:43

marko-radosavljevic self-requested a review September 18, 2021 19:44

marko-radosavljevic previously approved these changes Sep 18, 2021

View reviewed changes

marko-radosavljevic reviewed Sep 18, 2021

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/CommandHandler.java Outdated Show resolved Hide resolved

I-Al-Istannen previously approved these changes Sep 18, 2021

View reviewed changes

marko-radosavljevic dismissed stale reviews from I-Al-Istannen and themself via 28515f8 September 18, 2021 20:02

marko-radosavljevic self-requested a review September 18, 2021 20:03

d3vel0per reviewed Sep 18, 2021

View reviewed changes

Zabuzard previously approved these changes Sep 19, 2021

View reviewed changes

marko-radosavljevic approved these changes Sep 19, 2021

View reviewed changes

marko-radosavljevic requested changes Sep 19, 2021

View reviewed changes

Tais993 dismissed Zabuzard’s stale review via 0424e3f September 19, 2021 11:46

marko-radosavljevic self-requested a review September 19, 2021 11:48

marko-radosavljevic approved these changes Sep 19, 2021

View reviewed changes

Tais993 marked this pull request as draft September 20, 2021 07:47

Zabuzard approved these changes Sep 21, 2021

View reviewed changes

d3vel0per mentioned this pull request Sep 26, 2021

Add tophelpers command 🏆 #117

Closed

Zabuzard added the priority: major label Sep 27, 2021

Tais993 marked this pull request as ready for review September 28, 2021 11:12

Zabuzard closed this Oct 8, 2021

Zabuzard force-pushed the fix-issues-command-system branch from 0424e3f to 4e2b591 Compare October 8, 2021 07:22

This was referenced Oct 8, 2021

Multithreading for command system #169

Merged

Error reporting if bot added with wrong scope #170

Merged

Uh oh!

Fixed command system issues #119

Fixed command system issues #119

Uh oh!

Conversation

Tais993 commented Sep 15, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tais993 commented Sep 16, 2021

Uh oh!

Uh oh!

I-Al-Istannen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tais993 commented Sep 18, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marko-radosavljevic left a comment

Choose a reason for hiding this comment

Uh oh!

Tais993 commented Sep 20, 2021

Uh oh!

Zabuzard commented Sep 27, 2021

Uh oh!

Tais993 commented Sep 28, 2021

Uh oh!

Zabuzard commented Oct 8, 2021

Uh oh!

Zabuzard commented Oct 8, 2021

Uh oh!

Uh oh!