draft proposal for reorganizing command handling

[skybloo]

Before I got too crazy I wanted to suggest this as a template for redoing how the commands are handled. It could streamline reading it a heck of a lot, would make it easier to add aliases (just add another case statement under the current one). Would love feedback

[liquidnya]

I think moving the commands into their own functions could be a good idea.
But maybe having all this code and functions in one file is getting a lot (as it currently is on the master branch).
I think changing the if-else's to a switch-case might make it more difficult to follow control flow, because a gigantic switch-case containing a lot of commands might be a bit complex.

I think a good way of handling commands better would be to use a more declarative approach.
For example defining commands somewhere and putting them in a Hash-Map, Object or other data structure where the commands can be looked up and then a function is called that was previously registered in the data structure.
For example a very similar approach to what a discord.js guide is recommending for slash commands: https://discordjs.guide/interactions/slash-commands.html#guild-commands
E.g. each command could live in it's own file (or maybe a group of related commands in one file), and then it could be loaded at start-up and then when chat commands are detected (a leading !), the responsible function could be called.

Furthermore I have another suggestion on how to make command handling better.
This suggestion works regardless of which approach is taken:
Commands could in general be configured (e.g. in settings.json or maybe a new file command-mappings.json or so),
for example you could configure the list command to be !list, !queue or !status etc. and customize each command.
This way someone could change every command to their choosing and also deactivate commands, e.g. configuring an empty list for a command.
I have a working branch of this feature already: https://github.com/liquidnya/fluid-queso-queue/tree/feature-command-config
But I haven't ported it to this version of the queue yet, since other commands have been added and the code changed a lot since back then.
An example configuration of commands could look like this:
https://github.com/liquidnya/fluid-queso-queue/blob/feature-command-config/settings.js#L12-L39
I think this feature could be considered when reworking command handling, so it's build in from the beginning.

[skybloo]

something a little more like this? I have the aliases mapped the opposite of yours, just to make the code a little cleaner, though it wouldn't be too difficult to flatten as part of the startup process

[liquidnya]

I think it would be best to still pass the respond function to the individual command function. I guess it is just taste if respond is passed or if a string is returned, but at least for the test cases it is more traceable where the response was generated, because calling the respond function will generate a stack trace that can be printed in case the test fails, and when returning a string from the function that entry in the stack trace is missing. I also think commands then also have more control over maybe not even emitting text or emitting multiple chat messages.

[skybloo]

honestly I like the more functional style this promotes and it's what I went with in my first iteration so happy to agree

[liquidnya]

I was thinking of another use case with this approach:
The response function could also have have optional further arguments to indicate using a twitch chat command, or an announcement message or a reply to a message in chat etc., having this optional argument makes it easier to escape data properly when sending messages to chat.

[liquidnya]

I think the bot not having a response (instead of printing that the command is not valid) on any other command would be the way to go, since people don't want chat to be spammed and also might have other bots who answer on other commands.

[liquidnya]

The fallback could be a function that gets passed the response function (see other comment on line 242) that will then not do anything (no-op).

[liquidnya]

With this approach one could never rename the !add command to something else or disable the !add command.
I think it would be better to distinguish between command names and commands (and their aliases).
For example the dummyMapping could map a command name to a function, and then there is another map, that maps commands and aliases to command names:
For example there is the add command with the addLevel function, and then there is a !add and !push mapping for the add command.
With this approach the add command can also be configured to only be used with !push.
(add might be a bad example, but someone might want to disable the !customcodes command or the !queue command)

[skybloo]

agreed. I left the bangs out because I just don't like them tbh and it lets it shortcircuit non command messages faster

[liquidnya]

command names and commands were just terms I came up with to explain better what I mean,
maybe calling it function names and commands would be better,
like commands could be mapped to function names and those are mapped to the actual functions,
whatever definition works
Yeah not having the bangs in there is probably better, but I left them in my explanation, because I thought I could bring across my point better