Initial Commit Thoughts

I've pushed the initial commit for interaction handlers: heavily wip, totally untested and raw (just how I like it)! https://github.com/sapphiredev/framework/commit/e158f7f0bc9b17f4802f1c9a82abf03dc31c8776 Please take a look at it, leave thoughts (especially you @Developers smh), see what's unclear and what's clear, and list any final thing you'd rather see changed because this interface is nearing it's final state 😋 ⚠️ THIS DOES NOT INCLUDE APPLICATION COMMANDS YET!!! ⚠️ THOSE WILL COME AT A LATER TIME.

GitHub

feat(InteractionHandlers): initial commit 👀 🎉 · sapphiredev/framewo...

vladdy•966d ago

CC @.yugen. @RedS @Favna @noftaly @Deleted User @undiedgfx oh and @kyra 🩵🩷🤍🩷🩵 maybe they'll actually review >.>

𝖄𝖚𝖌𝖊𝖓•966d ago

UH OH Lemme finish this batch of lubing

Elliot•966d ago

i have a 2h lecture about linked list (read: 2h of free time) in 5min, so i'll look into this 🙂

RedS•966d ago

vladdy•966d ago

wanky

UndiedGFX•966d ago

@vladdy so i thought you pushed that into main branch and as you said its huge WIP i thought it would be buggy and @next wont work as expected by mb so yes thats why i said i cant install @next now

Elliot•966d ago

Why goerr rather than the monads you use everywhere in sapphire ? The second example for the parse method should be way simpler (why discard the first result of split? and maybe replace container.prisma with a more abstract thing such as database.findFirst()) otherwise looks fine, but i'm not really good at reviews (and haven't had the time to test it)

vladdy•966d ago

why goerr Simpler interface for me instead of having to wrap it in a try-catch every time making messy code, but if there's better methods I can change that

why discard the first result of split?

Sure.. the examples could be improved, but right now it's very much a wip, so even examples will change, but noted noted

Favna•966d ago

doubt it considering I told him to re-review the utilities PR 5x and he did nutting

UndiedGFX•966d ago

😂

RedS•966d ago

https://reds.is-inside.me/mox9ReZn.png

Unknown User•966d ago

Message Not Public

vladdy•966d ago

yeah, I can remove those later

Unknown User•966d ago

Message Not Public

vladdy•966d ago

Boo hoo

Unknown User•966d ago

Message Not Public

vladdy•966d ago

Oh nooo what will I ever do allocating an array for the sake of my convenience

Unknown User•966d ago

Message Not Public

vladdy•966d ago

There's absolutely nothing wrong with it

Unknown User•966d ago

Message Not Public

vladdy•966d ago

And? its nanoseconds You need better arguments than hurr durr muh nanoseconds

Unknown User•966d ago

Message Not Public

vladdy•966d ago

I am not making the code messier by wrapping it in a try catch just to please your nanosecond thirst

Unknown User•966d ago

Message Not Public

vladdy•966d ago

???? Wtf do you suggest I do then

Unknown User•966d ago

Message Not Public

vladdy•966d ago

Nah, you're the one complaining about something thats perfectly fine, so propose a solution

Unknown User•966d ago

Message Not Public

vladdy•966d ago

🙄 you really need to get over the speedwhoring btw

Unknown User•966d ago

Message Not Public

vladdy•966d ago

No, if you really wanted to have an argument I wouldn't have bitched at you over you could've phrased it much better than "hurrr durrr it allocates!!!! MY SPEED!!!1111!!!" Like Fuck me I get you love speed, god knows I do too

Unknown User•966d ago

Message Not Public

vladdy•966d ago

but stop making your argument revolve strictly on it

Unknown User•966d ago

Message Not Public

kyra•966d ago

I actually thought about making Result.from(cb) and Result.fromAsync(cbOrPromise) tbh

vladdy•966d ago

Maybe if you read what the package did you wouldn't say that but of course you wouldn't do that because oh my god!! oh MY GOD! it allocates an array!! guys look!!!1

Unknown User•966d ago

Message Not Public

kyra•966d ago

Also, since the code is most likely simple, the code is also most likely inlined So extra allocations are avoided thanks to later optimisations

vladdy•966d ago

export default function goerr(parameter: any): any {
  if ('then' in parameter && 'catch' in parameter) {
    return parameter
      .then((r: any) => [null, r])
      .catch((err: Error) => [err, null])
  }

  try {
    const result = parameter()
    return result instanceof Promise ? goerr(result) : [null, result]
  } catch (err) {
    return [err, null]
  }
}

export default function goerr(parameter: any): any {
  if ('then' in parameter && 'catch' in parameter) {
    return parameter
      .then((r: any) => [null, r])
      .catch((err: Error) => [err, null])
  }

  try {
    const result = parameter()
    return result instanceof Promise ? goerr(result) : [null, result]
  } catch (err) {
    return [err, null]
  }
}

Unknown User•966d ago

Message Not Public

vladdy•966d ago

Was showing it to kyra

kyra•966d ago

ye, most likely inlined Those types are absolutely horrible, Vlad

vladdy•966d ago

That..I have to agree with OMEGALUL

beg to mkfucking differ but ok meguFace

Unknown User•966d ago

Message Not Public

kyra•966d ago

@vladdy fun fact The change you did to optimise, actually de-optimised

kyra•966d ago

Because this is now part of the error handler block, where everything turns slower

kyra•966d ago

Also, why are you randomly bumping all deps, including typedoc?

kyra•966d ago

This should be overloaded,

public some(): Maybe<undefined>;
public some<T>(data: T): Maybe<T>;
public some<T>(data?: T): Maybe<T | undefined> {
  return some(data);
}

public some(): Maybe<undefined>;
public some<T>(data: T): Maybe<T>;
public some<T>(data?: T): Maybe<T | undefined> {
  return some(data);
}

𝖄𝖚𝖌𝖊𝖓•966d ago

Hey Go reply on the commit N00b

Lioness100•966d ago

I'm just so excited there's progress and this already looks so amazing and promising so thanks alot @vladdy for putting in the hard work blobheart

vladdy•966d ago

Are you fucking me

kyra•966d ago

Perhaps

vladdy•966d ago

the change to please doge is worse than the original code .w.

kyra•966d ago

vladdy•966d ago

End me How tho Safety Will do when I get back

Favna•966d ago

Dropped a bunch of comments Vladdy also tagged you in one with a question @kyra 🩵🩷🤍🩷🩵

vladdy•966d ago

I saw, I'll answer when I have a laptop

Favna•966d ago

np it's late

vladdy•966d ago

What I can say rn is that the allSettled won't change I want all handlers to run, not it to stop after the first handler errors I have a flight so

RedS•965d ago

Lioness100•965d ago

Also I think a Result.from[Async] would be really cool tbh

vladdy•957d ago

@kyra 🩵🩷🤍🩷🩵 work on that pls I don't mind it, quite the opposite I wanna get it right in one PR @kyra 🩵🩷🤍🩷🩵question

        for (const handler of this.values()) {
            // TODO: this really needs to be optimized, but I don't know how right now :c
            const filter = InteractionHandlerFilters.get(handler.interactionHandlerType);

        for (const handler of this.values()) {
            // TODO: this really needs to be optimized, but I don't know how right now :c
            const filter = InteractionHandlerFilters.get(handler.interactionHandlerType);

can this be optimized at all? or is it fine

kyra•957d ago

That's O(n) So can't be optimised more

vladdy•957d ago

Ah kek

        for (const handler of this.values()) {
            const filter = InteractionHandlerFilters.get(handler.interactionHandlerType);

            // If the filter is missing (we don't support it / someone hasn't registered it manually while waiting for us to implement it),
            // or it doesn't match the expected handler type, skip the handler
            if (!filter?.(interaction)) continue;

            let result: Maybe<unknown>;

            try {
                // Get the result of the `parse` method in the handler
                result = await handler.parse(interaction);
            } catch (err) {
                // If the `parse` method threw an error (spoiler: please don't), skip the handler
                // TODO: Emit an event (interactionHandlerParseError) that the parse method errored out
                continue;
            }

            // If the `parse` method returned a `Some` (whatever that `Some`'s value is, it should be handled)
            if (isSome(result)) {
                // Schedule the run of the handler method
                promises.push(handler.run(interaction, result.value));
            }
        }

        for (const handler of this.values()) {
            const filter = InteractionHandlerFilters.get(handler.interactionHandlerType);

            // If the filter is missing (we don't support it / someone hasn't registered it manually while waiting for us to implement it),
            // or it doesn't match the expected handler type, skip the handler
            if (!filter?.(interaction)) continue;

            let result: Maybe<unknown>;

            try {
                // Get the result of the `parse` method in the handler
                result = await handler.parse(interaction);
            } catch (err) {
                // If the `parse` method threw an error (spoiler: please don't), skip the handler
                // TODO: Emit an event (interactionHandlerParseError) that the parse method errored out
                continue;
            }

            // If the `parse` method returned a `Some` (whatever that `Some`'s value is, it should be handled)
            if (isSome(result)) {
                // Schedule the run of the handler method
                promises.push(handler.run(interaction, result.value));
            }
        }

is this better? related to https://canary.discord.com/channels/737141877803057244/891321859650510878/891426667887747152

kyra•957d ago

const result = await Result.makeAsync(handler.parse(interaction));

const result = await Result.makeAsync(handler.parse(interaction));

vladdy•957d ago

we don't have that pepeHands

kyra•957d ago

We do Rebase

vladdy•957d ago

i.. for fuck sake

vladdy•957d ago

@Favna https://github.com/sapphiredev/framework/commit/e158f7f0bc9b17f4802f1c9a82abf03dc31c8776#r57062331 Not changing as I want all errors reported, not just the first one

GitHub

feat(InteractionHandlers): initial commit 👀 🎉 · sapphiredev/framewo...

vladdy•957d ago

https://github.com/sapphiredev/framework/commit/e158f7f0bc9b17f4802f1c9a82abf03dc31c8776#r57061938 I'm leaning towards camelCase, thoughts? @Developers

GitHub

feat(InteractionHandlers): initial commit 👀 🎉 · sapphiredev/framewo...

𝖄𝖚𝖌𝖊𝖓•957d ago

Inb4I have to update my plugin

Favna•957d ago

The store for scheduled tasks in the plugin will be scheduled-tasks as well in the current pe The store for scheduled tasks in the plugin will be scheduled-tasks as well in the current pr I'll let @kyra 🩵🩷🤍🩷🩵 make the final call Vladdy. I can go either way. But if we do go camelCase then yes Yugen's PR should be adjusted.

kyra•957d ago

scheduled-tasks I'm not a fan of having uppercase characters

Favna•957d ago

So also interaction-handlers then pikaCool