Sapphire - Imagine a framework•4y ago

Preconditions with missing handlers! What do?

Preconditions: What should happen with preconditions (global or not) that need to run on a message command but do not have a handler for them? Skip? Throw error?

43 Replies

UndiedGFX•4y ago

Maybe skip them till you add preconditions for slashies?

vladdyOP•4y ago

Not the question at hand

24•4y ago

can you define an example?

vladdyOP•4y ago

Preconditions will happen across the board, the question is what should happen if a command that can run both via / and message receives a precondition that only handles /

24•4y ago

skip means you could define everything in 1 array buuut

vladdyOP•4y ago

but should it be skipped Or should it throw an error

UndiedGFX•4y ago

Maybe throw an ephermal message? As error

vladdyOP•4y ago

Not doing user responses for them

UndiedGFX•4y ago

vladdyOP•4y ago

We don't even have an in-chat error message by default

24•4y ago

IMO skipping them means you have to deal with mixed preconditions (slashes and messages) which can result in either nasty nested arrays, or w/e

UndiedGFX•4y ago

I mean we do handle it On commandDenied

vladdyOP•4y ago

There is no listener for commandDenied included by default

UndiedGFX•4y ago

So just like how normal preconditions work would be OK ish Erhm

vladdyOP•4y ago

I'm also not talking about the preconditions array But running the preconditions specified

UndiedGFX•4y ago

Maybe throwing an error could be helpful

24•4y ago

can you define an example? I still don't quite follow exactly

vladdyOP•4y ago

Pretty sure the question speaks for itself.. Take this precondition

import type { Message } from 'discord.js';
import { Identifiers } from '../lib/errors/Identifiers';
import { Precondition, PreconditionResult } from '../lib/structures/Precondition';

export class CorePrecondition extends Precondition {
    public chatInputRun(message: Message): PreconditionResult {
        return message.guild === null
            ? this.ok()
            : this.error({ identifier: Identifiers.PreconditionDMOnly, message: 'You cannot run this command outside DMs.' });
    }
}

import type { Message } from 'discord.js';
import { Identifiers } from '../lib/errors/Identifiers';
import { Precondition, PreconditionResult } from '../lib/structures/Precondition';

export class CorePrecondition extends Precondition {
    public chatInputRun(message: Message): PreconditionResult {
        return message.guild === null
            ? this.ok()
            : this.error({ identifier: Identifiers.PreconditionDMOnly, message: 'You cannot run this command outside DMs.' });
    }
}

UndiedGFX•4y ago

What vlad says it that it isn't handler for interactions What should he do at that point

vladdyOP•4y ago

Now let's assume your command that says this precondition is needed handles message based commands AND slash commands

UndiedGFX•4y ago

But it isn't handled for slash commands

vladdyOP•4y ago

Or messages or contextMenus

UndiedGFX•4y ago

Because maybe you forgot to make a handler I mean aren't they handled by sapphire for messages?

vladdyOP•4y ago

message handlers will be optional

UndiedGFX•4y ago

vladdyOP•4y ago

and well chatInputRuns are considered the new abstract run you must always provide

24•4y ago

that makes more sense, I couldn't quite think the way through the question. I think in these cases, if we error them, we would then have more runtime errors (unless they can be typed) If we skip them, we could run into some conditions that would appear to be running, but don't, thus causing frustration for an end user I would accept having them error due to having a much better understanding of why a precondition didn't run (If it errored, I didn't provide a valid handler / I used the wrong precondition- that should error, makes it much easier to debug and better (D)UX)

vladdyOP•4y ago

I'm leaning towards hard erroring because it's safer for the user

24•4y ago

I'd agree

UndiedGFX•4y ago

I'd agree too

24•4y ago

I can see skipping becoming a hassle for people in #old-sapphire-support i i.e. "Why did my precondition not run for interactions but do for messages"

vladdyOP•4y ago

or permission escalations

24•4y ago

yup I don't see any valid use cases for skipping it just seems like it would cause more headaches for everyone (devs, you)

vladdyOP•4y ago

But then SHOULD the chatInput ones be mandatory or not.. I want to make chatInputCommands the pushed way bc well, they're nice

24•4y ago

good question in doing so you would force people to think about using slash commands over message commands win-win right?

vladdyOP•4y ago

which is what Discord (and ngl I) want

24•4y ago

I can see that being annoying for some users but really, really important down the road

vladdyOP•4y ago

I mean the bitching will only bitch for TS users

24•4y ago

:) yeah I can see this being slightly annoying at first, but nothing a find and replace command can solve

𝖄𝖚𝖌𝖊𝖓•4y ago

If no handler is defined. Dont run it. I think a same approach like the commands would be beneficial

vladdyOP•4y ago

Yeah BUT it can mean permissions escalation Or accidentally forgetting to handle it

𝖄𝖚𝖌𝖊𝖓•4y ago

I mean. Skipping error'ing is just as bad And since commands already have a seperate function to handle slashcommand functionality. It wouldnt be weird to have the same logic apply to other pieces imo

vladdyOP•4y ago

..well, yes

Gaming

Programming

Preconditions with missing handlers! What do?