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
CC @.yugen. @RedS @Favna @noftaly @Deleted User @undiedgfx
oh and @kyra π©΅π©·π€π©·π©΅ maybe they'll actually review
>.>
UH OH
Lemme finish this batch of lubing
i have a 2h lecture about linked list (read: 2h of free time) in 5min, so i'll look into this π
wanky
@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
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)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
doubt it considering I told him to re-review the utilities PR 5x and he did nutting
π
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
yeah, I can remove those later
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
Boo hoo
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
Oh nooo what will I ever do allocating an array for the sake of my convenience
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
There's absolutely nothing wrong with it
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
And?
its nanoseconds
You need better arguments than hurr durr muh nanoseconds
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
I am not making the code messier by wrapping it in a try catch just to please your nanosecond thirst
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
????
Wtf do you suggest I do then
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
Nah, you're the one complaining about something thats perfectly fine, so propose a solution
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
π
you really need to get over the speedwhoring btw
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
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β’4y ago
Message Not Public
Sign In & Join Server To View
but stop making your argument revolve strictly on it
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
I actually thought about making
Result.from(cb)
and Result.fromAsync(cbOrPromise)
tbhMaybe 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β’4y ago
Message Not Public
Sign In & Join Server To View
Also, since the code is most likely simple, the code is also most likely inlined
So extra allocations are avoided thanks to later optimisations
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
Was showing it to kyra
ye, most likely inlined
Those types are absolutely horrible, Vlad
That..I have to agree with my beg to mkfucking differ but ok
Unknown Userβ’4y ago
Message Not Public
Sign In & Join Server To View
@vladdy fun fact
The change you did to optimise, actually de-optimised
Because this is now part of the error handler block, where everything turns slower
Also, why are you randomly bumping all deps, including typedoc?
This should be overloaded,
Hey
Go reply on the commit
N00b
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
Are you fucking me
Perhaps
the change to please doge is worse than the original code
.w.
End me
How tho
Safety
Will do when I get back
Dropped a bunch of comments Vladdy
also tagged you in one with a question @kyra π©΅π©·π€π©·π©΅
I saw, I'll answer when I have a laptop
np
it's late
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
Also I think a
Result.from[Async]
would be really cool tbh@kyra π©΅π©·π€π©·π©΅ work on that pls
I don't mind it, quite the opposite
I wanna get it right in one PR
@kyra π©΅π©·π€π©·π©΅question
can this be optimized at all?
or is it fine
That's O(n)
So can't be optimised more
Ah
kek
is this better?
related to https://canary.discord.com/channels/737141877803057244/891321859650510878/891426667887747152
we don't have that
We do
Rebase
i..
for fuck sake
@Favna
https://github.com/sapphiredev/framework/commit/e158f7f0bc9b17f4802f1c9a82abf03dc31c8776#r57062331
Not changing as I want all errors reported, not just the first one
https://github.com/sapphiredev/framework/commit/e158f7f0bc9b17f4802f1c9a82abf03dc31c8776#r57061938
I'm leaning towards camelCase, thoughts? @Developers
Inb4I have to update my plugin
The store for scheduled tasks in the plugin will be
scheduled-task
s as well in the current pe
The store for scheduled tasks in the plugin will be scheduled-task
s 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.scheduled-tasks
I'm not a fan of having uppercase characters
So also
interaction-handlers
then E w
a dash??
Well that's what was in my comment innit
i'd rather go the python land route and use snake_case than
Nobody uses _ in filenames
Beg to differ
And they don't work nicely with Ctrl + arrows
??
Control + arrow navigation skips over _ as it treats it as 1 word whereas - stops at the hyphen.
and that matters how
It's annoying when something that are actually 2 words is treated as 1 when using keyboard i.e. vim navigation
Because it means using individual arrow keys which is a bother
https://github.com/sapphiredev/framework/commit/1c3dd2674bc48795cfecebf541ca55d756c365cf @Favna @kyra π©΅π©·π€π©·π©΅ happe?
"a link or a promised link"
Let's return something else because breaking promises is fun /s
Anyway, yes, happe