We've reached an agreement and can close this and get on with the developing! \o/ :tada:

from cactusbot.

@Innectic and I came to an agreement internally:

We merge !cactus, !test, and !help into one magic command, to ensure that the bot always contains a link to our documentation and such.

We leave the option for generating defaults, for easter eggs, automatic aliasing (!cactus docs -> !help), etc.

@RPiAwesomeness: Thoughts?

from cactusbot.

@Innectic @RPiAwesomeness If we can get three 👍 on this, we're set! 🎉

Magic Commands

These stay as they were in my proposal. We keep them in the bot itself, but ensure the API can return sufficient information about them.

Builtin Commands

These are removed, and replaced with defaults through the API, as described in my proposal.

Defaults only exist once in the database, to save space, and to ensure that defaults which are newer than a user are still accessible by that user. They may be overriden, but when the custom override is !command removed, they are accessible again.

We should also make it possible for a user to be able to toggle the use of defaults in their channel through an endpoint in the API (and eventually a magic command).

from cactusbot.

I vote handling it in the bot still.

from cactusbot.

First off, that's a SUUUUUPER unlikely application of the API. And I don't think I entirely understand your example. Are they using CB as the other user, as in the whole bot? Or are they just using the API? If they're just using the API, then they're literally just using us as a storage facility, which isn't a proper use of the API and shouldn't be something we allow.

If they wanted to use CB to create a chat thing or whatever, they would have to write their own handler that recognizes the command creation command, parses it, and makes the proper request to the proper endpoint with the proper information. Our API shouldn't be required to figure out what's it's supposed to be storing. An API should only be doing CRUD actions. Create/Read/Update/Delete. And it shouldn't be making any requests to an outside service unless ABSOLUTELY necessary, that sort of stuff should be handled by the client and then passed to the server. Innectic and xCausxn both agree with me on this.

1. This makes no sense for any developer to spend the extra time to make a request through another service. Why have 5 requests when they can do it in 1? And yes, it would take 5 HTTP requests/responses. Client -> API -> Outside service API -> response to API -> response to client.

2. This is to protect us. We don't want to get rate-limited/banned by any service because we were requesting stuff too fast, which could easily be a problem if we do something like !social on the API.

We don't need to "release" the API. It just is. If we make major changes, we push them and tag a quick release with a mention of the CB release it was written to work with. No need for paired releases. People will rarely be self-hosting the API, if ever.

Yes, the request times will be very minor. HOWEVER, that IS more time for a command that can & should be nearly instantaneous.

As for people not knowing how our stuff works, that's why we write docs and have a Twitter/GitHub/Discord. Although it may be annoying for a developer to have to make changes in their code every time we update, this is normal practice for anyone utilizing an outside service. Either you deal with updates and changes or don't use the service. We do this with Beam, and will eventually do it with Twitch, Discord, etc.

In all honesty, I think we're overthinking what's going on. How many people have we had that want to override !cactus? Zero that I can think of. How many people have we had that have wanted to use CB for anything other than a bot? Zero that I can think of. Yes, it might happen in the future, but it hasn't happened yet (AFAIK) and it is doubtful it would happen any time soon. We're not that big, and probably won't be for a while, if ever.

As for leaving it open, this is development. Yes, we should try and make something that is future-proof, but that's literally impossible because things are constantly changing. So, we do the best we can now, and if a need arises in the future, then we make changes then.

Finally, the API is really for our use. The API should be used for accessing quotes/custom commands/friends/settings/etc. for the individual bot instances. Different people using the same codebase, but with different individual settings & stuff. If something is going to be the same between all the different instances, then it shouldn't require the API to work. Like !cactus or !temmie. These responses (at least for now, another discussion was had/may be had again about this) are the same across all instances, unless someone goes in and edits code, which is beyond our control. If people want to utilize the API, they need to be doing it simply to get info about quotes/commands/settings/etc. for different bots, not using our resources to create responses to their stuff.

Sorry for the crazy long-winded response 😛

from cactusbot.

Here's what I've gathered from the +1's and the responses:

The api doesn't know about the built-in commands, and the bot still handles it, like it already does. (And that we're settling with what @RPiAwesomeness said)
And, @RPiAwesomeness :
No one has asked me for an override for any of the built-ins. So, you're correct in saying that :D

from cactusbot.

I vote using the API to deal with them.

Custom commands are handled through the API, and GETting /command/custom returns the response, and information about the command. However, if we keep builtin/magic commands in the bot itself, it seems to developers that these commands do not exist (according to the API), and it is impossible to get a response for them.

It won't make sense to users that the API doesn't know about builtins/magics.

• From a developer PoV, "Why do I have to update my code manually to support your new builtins every time you update? Shouldn't a query to the API tell me whether or not a command exist, and return its response?"
• From a user PoV, "Why doesn't work for the commands that come with the bot, but it does work for commands I add?" (This also will slow future development - we'll have to write any feature that deals with commands three times - one for each type of command.)

from cactusbot.

My thinking is that the API should return some usage for builtins because, like you say @2Cubed, it's confusing to developers if the command works in-bot, but if they try and access the endpoint, then it doesn't work.

So, we compromise, and I think this solution works out for the better. The API shouldn't really be going out and getting and parsing information, just getting pre-parsed info from the client, and then passing the proper data back on request.

So, my thinking is the bot handles all of these commands in-bot and the API, when requested by a third-party client, returns usage for that command (and maybe some links to code on GitHub related to that command?), but doesn't actually create the response that CB would send. It doesn't have a need to go through the API, that will just slow things down (minorly, but still) and give more chances for breakage.

If someone is utilizing CB for their own project (which is unlikely, at least for awhile), they're most likely not utilizing it for the magic commands. For example, someone who wants social information about a user isn't going to go through !social and take 5 requests to get what they want when they can do it in 1 request in their own code. And no-one will need to request an API just to get back Ohai! I'm cactusbot :cactus (!cactus).

If this becomes an issue, we can always change things later.

from cactusbot.

@RPiAwesomeness I think that a good idea. The api knows about it, but the bot doesn't request it. I think that's a good way to do it. I really didn't like the idea of having to request the response for a command who's response won't change.

from cactusbot.

@RPiAwesomeness Hmm... interesting.

While I agree that developers could be confused by the lack of API response, it's not just the humans that would be confused - the code is a big issue, too.

Consider a developer who is writing a simple web panel that is, essentially, a chat between the user and CactusBot. (You run a command, it responds - just like a service chat, but with CactusBot being the only other user, ever.) Whenever you run a command, it GETs /command/:command.

The user runs !command add x y, expecting the command to add. (Their code just displays packet["data"]["response"], or whatever.) Instead, the API returns a 404, packet["data"]["response"] doesn't exist, and their code crashes. "What did I do wrong? 🤔 " (Maybe packet["data"]["usage"] exists... or maybe packet["data"]["response"] exists, but contains an error/usage message, rather than an actual response.)

To solve this problem, they would have to hard-code the responses to every single builtin and magic command into their code, as well. If we update CactusBot and add a !raid magic command, their code breaks for that command, until they update to support what we changed in the bot, even though they're just using the API.

I see this as a pretty large issue. In general, it's bad coding practice to put "things" that do the same "stuff" in different places, and for all intensive purposes from a user's or developer's perspective, all commands are created equal, magic, builtin, or custom.

I agree that the API shouldn't be handling any parsing - we should leave that to the connections (official CactusBot or custom code).

It should be noted that, in addition to the developer pain it would cause, pairing magic commands with both the bot itself and the API would force us to also pair release schedules for the two, which could get quite annoying if we need to change things on either side. (It would also be confusing as to which version of the bot and the API "match".)

I don't think the argument of speed applies here, to be honest - request times should be of the order of milliseconds. Also, it's more likely that things would break inside of the bot if we kept the magic commands within it; if we do that, we would have to be handling command responses in three different ways, which could easily cause some nasty bugs.

I would disagree - commands are a key part of the bot, and having commands with custom behaviors is a pretty big deal, IMHO. Also, !social information would only take one request to the API - GET /command/social would return the desired response.

@Innectic

Not needing to make a request to get the !cactus response may seem logical at first, but really, it's not much different from making a request for a custom command - both just return a response. (I still think we should replace "built-ins" with overrideable "defaults", but we can talk about that in another issue, if need be.)

To a developer who doesn't know that the response to !cactus is hard-coded, especially, the only difference would be negative - while it's true that we could "predict" that !cactus returns the response that it does, there's no indication of that to anyone using the API. Making them hardcode it into their application, too, seems extremely silly.

If it would make you happy, we could compromise by doing "both" - we could put magics and builtins behind the API, but have our code do the following:

BUILTINS = {
    "cactus": "Ohai! I'm CactusBot. 🌵"
}

if command in BUILTINS:
    return BUILTINS[command]
else:
    return cactus_api.get_command(command)["response"]

This seems rather redundant, though.

Finally, I don't think this is something we should leave open to changing later; right now, we need a solution which will last us for many releases, and we shouldn't plan that solution around changing it later.

from cactusbot.

@RPiAwesomeness

Hmm, good point. I guess that does make sense.

1. I mean... not quite. Requests and responses are "paired", so it would only be two, but... yeah.
2. Huh, I hadn't thought about the fact that !social needs to access external API's. Very good point.

We should be doing actual releases for the API, rather than just loading random versions onto the server. But I guess it's fair that most people will probably use the hosted API, rather than hosting their own.

Meh. It's milliseconds - nobody's going to notice. I don't see speed as an issue.

We only change things when Beam (and other services) make developer-related changes, not, say, adding a new chat /command. If we do update the API to reflect at least the existence of new magic commands, though... I guess that's fine.

OK... that makes sense. I think I'm on board with this.

Proposal

Magic Commands

The API should:

• Return usage.
• Return a short "docstring"-esque description of what, in human terms, the command does.
• Include a type attribute for every command, indicating either custom or magic.
• Return all magic commands in /commands. (Useful for autocompletion, among other things.)

Builtin Commands

These should not exist. It makes sense that magic commands should have custom responses and behaviors, but builtins are just text responses: the equivalent of custom commands, but just hard-coded.

Instead, we should write some code in the API which generates defaults, which are exactly equivalent to custom commands, upon the initialization of each bot instance, based on a JSON file. The file would publicly contain responses for !test, !help, and !cactus, but we could privately add responses for, say, easter eggs. These defaults can be overwritten, but if the custom overrides are !command removed, the default is restored.

!test
Test confirmed. 🌵

!command add test This was a triumph...
Command added.

!test
This was a triumph...

!command remove test
Command removed.

!test
Test confirmed. 🌵

from cactusbot.

I disagree with your proposal. Builtin command should not be overridable. Lets make a scenario:

I'm playing JerkyMcJerkFace.

!command add cactus CactusDev smells like poop!
!command add test CactusBot sucks potato!

We're basically allowing people to be mean. Yes, I understand not everyone is like that, but those people still exist. We need to keep that in mind. I wouldn't want to join a chat and see anything like that.

Ok, I'm Innectic again.

I see no reason that usage, or a docstring for them is needed. We have documentation. If people are too lazy to not read them, that's their fault for being dumb.

No reason to specify what type of command it is if the bot just handles it. The type should be what it is. It's a command. Not a magic, or a custom. That's silly. Like silly string. Silly string is annoying. That would be too

from cactusbot.

@Innectic People can be "mean" in all sorts of ways. If they want to add custom commands that insult others, that's their own time wasted.

I'm playing BoatyMcBoatface, who happens to be a jerk.

!command add cactato CactusBot is a potato!11!!!111

Even if we disable the overriding of builtins, people can still accomplish the same thing in other ways.

OK, I'm 2Cubed again. 😛

• Usage would be good for things like command autocomplete, which is only possible if we know, for example, how many arguments a command needs. (We can automagically figure out how many arguments a custom command needs based on the maximum %ARGN% specified.)
• Docstring would be generally nice - in addition to it being helpful to human API users, it'd be nice for, say, !help quote.

Specifying what type of command is imperative. Going back to the developer using the API, if their code is expecting data["response"] and that doesn't exist, it's going to cause problems. They need to be able to check if data["type"] == "custom", and if not, do things differently.

from cactusbot.

@2Cubed I like that idea. Those were really the only major ones I could think of (aside from various easter-eggs, which are probably better in the API anyways, because of how often they may be changed). Those will be hard-programmed, only changing if necessary in a release? Otherwise, they get loaded from a local JSON file?

The biggest problem I see with overriding defaults is for !cactus/!test/!help because those are expected to be the same between all instances of CB and could easily be made offensive/dangerous. Someone could hijack the !help command and make it redirect to a minified malware site or something. I strongly don't think those should be overridable, but I guess others could be.

I also agree with @2Cubed in that it should return everything you said, docstring, usage, etc.

from cactusbot.

@RPiAwesomeness Good point - malicious links would be bad. If we update our docs to show that !cactus docs is the way to get to the docs, though, instead of !help, I think we're fine.

from cactusbot.

So, are we all in agreement?

from cactusbot.