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.
88 Replies
vladdy
vladdyOPβ€’4y ago
CC @.yugen. @RedS @Favna @noftaly @Deleted User @undiedgfx oh and @kyra 🩡🩷🀍🩷🩡 maybe they'll actually review >.>
π–„π–šπ–Œπ–Šπ–“
UH OH Lemme finish this batch of lubing
Elliot
Elliotβ€’4y ago
i have a 2h lecture about linked list (read: 2h of free time) in 5min, so i'll look into this πŸ™‚
RedS
RedSβ€’4y ago
wolfgalBongoFast
vladdy
vladdyOPβ€’4y ago
wanky
UndiedGFX
UndiedGFXβ€’4y 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
Elliotβ€’4y 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
vladdyOPβ€’4y 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
Favnaβ€’4y ago
doubt it considering I told him to re-review the utilities PR 5x and he did nutting
UndiedGFX
UndiedGFXβ€’4y ago
πŸ˜‚
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
yeah, I can remove those later
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
Boo hoo
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
Oh nooo what will I ever do allocating an array for the sake of my convenience
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
There's absolutely nothing wrong with it
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
And? its nanoseconds You need better arguments than hurr durr muh nanoseconds
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
I am not making the code messier by wrapping it in a try catch just to please your nanosecond thirst
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
???? Wtf do you suggest I do then
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
Nah, you're the one complaining about something thats perfectly fine, so propose a solution
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
πŸ™„ you really need to get over the speedwhoring btw
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y 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
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
but stop making your argument revolve strictly on it
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
kyra
kyraβ€’4y ago
I actually thought about making Result.from(cb) and Result.fromAsync(cbOrPromise) tbh
vladdy
vladdyOPβ€’4y 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
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
kyra
kyraβ€’4y 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
vladdyOPβ€’4y 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
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
vladdy
vladdyOPβ€’4y ago
Was showing it to kyra
kyra
kyraβ€’4y ago
ye, most likely inlined Those types are absolutely horrible, Vlad
vladdy
vladdyOPβ€’4y ago
That..I have to agree with OMEGALUL my shrug beg to mkfucking differ but ok meguFace
Unknown User
Unknown Userβ€’4y ago
Message Not Public
Sign In & Join Server To View
kyra
kyraβ€’4y ago
@vladdy fun fact The change you did to optimise, actually de-optimised
kyra
kyraβ€’4y ago
Because this is now part of the error handler block, where everything turns slower
No description
kyra
kyraβ€’4y ago
Also, why are you randomly bumping all deps, including typedoc?
kyra
kyraβ€’4y 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);
}
No description
π–„π–šπ–Œπ–Šπ–“
Hey Go reply on the commit N00b
Lioness100
Lioness100β€’4y 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
vladdyOPβ€’4y ago
Are you fucking me
kyra
kyraβ€’4y ago
Perhaps
vladdy
vladdyOPβ€’4y ago
wolfgalheart the change to please doge is worse than the original code .w.
kyra
kyraβ€’4y ago
yes
vladdy
vladdyOPβ€’4y ago
End me How tho Safety Will do when I get back
Favna
Favnaβ€’4y ago
Dropped a bunch of comments Vladdy also tagged you in one with a question @kyra 🩡🩷🀍🩷🩡
vladdy
vladdyOPβ€’4y ago
I saw, I'll answer when I have a laptop
Favna
Favnaβ€’4y ago
np it's late
vladdy
vladdyOPβ€’4y 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
RedSβ€’4y ago
No description
Lioness100
Lioness100β€’4y ago
Also I think a Result.from[Async] would be really cool tbh
vladdy
vladdyOPβ€’4y 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
kyraβ€’4y ago
That's O(n) So can't be optimised more
vladdy
vladdyOPβ€’4y 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
kyraβ€’4y ago
const result = await Result.makeAsync(handler.parse(interaction));
const result = await Result.makeAsync(handler.parse(interaction));
vladdy
vladdyOPβ€’4y ago
we don't have that pepeHands
kyra
kyraβ€’4y ago
We do Rebase
vladdy
vladdyOPβ€’4y ago
i.. for fuck sake
π–„π–šπ–Œπ–Šπ–“
Inb4I have to update my plugin
Favna
Favnaβ€’4y 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
kyraβ€’4y ago
scheduled-tasks I'm not a fan of having uppercase characters
Favna
Favnaβ€’4y ago
So also interaction-handlers then pikaCool
vladdy
vladdyOPβ€’4y ago
E w a dash??
Favna
Favnaβ€’4y ago
Well that's what was in my comment innit
vladdy
vladdyOPβ€’4y ago
i'd rather go the python land route and use snake_case than monkaW
kyra
kyraβ€’4y ago
Nobody uses _ in filenames
vladdy
vladdyOPβ€’4y ago
Beg to differ
kyra
kyraβ€’4y ago
And they don't work nicely with Ctrl + arrows
vladdy
vladdyOPβ€’4y ago
??
Favna
Favnaβ€’4y ago
Control + arrow navigation skips over _ as it treats it as 1 word whereas - stops at the hyphen.
vladdy
vladdyOPβ€’4y ago
and that matters how
Favna
Favnaβ€’4y ago
It's annoying when something that are actually 2 words is treated as 1 when using keyboard i.e. vim navigation
vladdy
vladdyOPβ€’4y ago
Sadge
Favna
Favnaβ€’4y ago
Because it means using individual arrow keys which is a bother
Favna
Favnaβ€’4y ago
"a link or a promised link" Let's return something else because breaking promises is fun /s Anyway, yes, happe
Want results from more Discord servers?
Add your server