/addcommand and /remove command #19
Conversation
Co-authored-by: Hokkaydo <42278077+Hokkaydo@users.noreply.github.com>
Co-authored-by: Hokkaydo <42278077+Hokkaydo@users.noreply.github.com>
Co-authored-by: Hokkaydo <42278077+Hokkaydo@users.noreply.github.com>
Co-authored-by: Hokkaydo <42278077+Hokkaydo@users.noreply.github.com>
|
It's a little mess of commits, normally it doesn't cause any problems, but if you want me to spend time cleaning it up, just say so ;) |
|
Adding & removing Discord side commands on the fly is not well supported by Discord. Reviewing tomorrow
Don't worry about that. Anyway, everything will get squashed into one single commit |
There was a problem hiding this comment.
Except for the few comments, it looks nice, many thanks
However, I have some worries about Discord API limitations.
A bot cannot add more than 200 commands a day.
Each time this system adds or removes a command, it adds the other N-1 commands.
Thus if someday we have ~50 commands (reachable IMO), we could encounter an error if we add or remove commands more than four times.
At first, I wanted to do that to remove commands of disabled modules. However, when I started testing the idea I quickly reached the limit.
The system is still really cool. We could merge it and keep in mind that if someday a bug about that occurs, it's maybe related here
|
|
||
| public class CustomCommand implements Command { | ||
|
|
||
| private String NAME; |
There was a problem hiding this comment.
fields should be in lowerCase and final
| public class CustomCommand implements Command { | ||
|
|
||
| private String NAME; | ||
| private String TEXT; |
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| System.out.println(commands); |
There was a problem hiding this comment.
Remove sysout or Logger.log if absolutely necessary
| public List<Command> getCommands() { | ||
| List<Command> new_commands = new ArrayList<>(); | ||
| commands.forEach((name, item) -> new_commands.add(new CustomCommand(name, item.content))); | ||
| System.out.println(new_commands); |
There was a problem hiding this comment.
Remove sysout or Logger.log if absolutely necessary
| StringBuilder commandContent = new StringBuilder(); | ||
| StringBuilder authorId = new StringBuilder(); | ||
| int index = 0; | ||
| while((c = stream.read()) != -1) { |
There was a problem hiding this comment.
Maybe use the BufferedReader here which allows you to split on "\n" using BufferedReader#readLine then split the line on ";" and use each of the three parts. Sounds easier to me but at your convenience
|
|
||
| CommandItem command = commands.get(commandName); | ||
|
|
||
| if(!command.authorId.equals(author.getId())) return; |
There was a problem hiding this comment.
Why this check?
Admins will not be able to delete a command 🤔
There was a problem hiding this comment.
I just tough it would be more logical that only the author of the command would be able to delete it ... but we can drop this mechanic and let anyone delete any CustomCommand
There was a problem hiding this comment.
Imo command creation & deletion was a thing only available to mods
|
|
||
| @Override | ||
| public List<OptionData> getOptions() { | ||
| return List.of( |
|
|
||
| @Override | ||
| public Supplier<String> getDescription() { | ||
| return () -> TEXT; |
There was a problem hiding this comment.
Here it could be better to add a description field in commands (and so in /addcommand)
implementing issue 16