#DSharpPlus.Commands

1 messages · Page 2 of 1

red notch
#

categories are really simple if we agree on the precondition of "only top-level commands can have a category"

#

then the command root can store the categories and you simply scan that list

#

if that precondition isn't met....

#

fun ensues (the definition of fun is disputed)

#

i could probably still do that but it really wouldn't be pretty and would probably have numerous weirdnesses

unique sparrow
#

I think this is going to be held back by pagination

#

Any decent category implementation imo

red notch
#

that's fine

#

we can do that later, we should just not pull a cnext and come up with a representation that's horrible to update

unique sparrow
#

Remember the pagination design I had a long time ago for @proud radish DocBot?

red notch
#

be it categories or not

#

distantly

unique sparrow
#

Let me see if I can find a screenshot

#

I want to do the two bars again. Buttons on the second row for when there's multiple pages, dropdown menu when there's categories

red notch
#

sounds good

#

ideally we'd also support paginating the dropdown just in case, but

#

i doubt anyone has a realistic use-case for 25 categories

unique sparrow
#

Not a case I'm wanting to support

#

At that point just make your own help command lol

#

OH THAT'S ANOTHER THING

#

I absolutely despise how the help command has so many components in CNext

red notch
unique sparrow
red notch
#

yeah the cnext help command manages to strike an impressive balance between being horribly overcomplicated and also being horribly rigid

unique sparrow
#

Honestly I really should've complained more about the category PR before it was merged

red notch
#

i think categories are really useful but we should take our time to do it properly

#

cnext categories are very much a bandaid; and i'm not opposed to deprecating cnext before we have that feature

#

as for making a help command design-wise, something similar to ICommandExecutor should work fine, no?

#

when we do ##1840 we also implicitly get support for composing a help command from multiple different things, ie having an extension layer over the default and then applying your formatting over the extension

proud radishBOT
red notch
#

uh

#

##1820

proud radishBOT
unique sparrow
#

Sounds complicated

red notch
#

not really

#

particularly because Commands gets that support more or less for free

unique sparrow
#

The concept sounds complicated

red notch
#

its scary but not that hard once you wrap your head around it

red notch
#

i'm kinda surprised to see people struggling with the ServiceProvider requirement of the extension

#

1820 isn't gonna be fun for them, but maybe we can figure out a way to make a simple hello world run without manual mention thereof

vernal acorn
#

Its not objective but imo it shouldnt be something we let stop us

red notch
#

no not at all

#

but if we can do that without going out of our way otherwise, that's probably a good idea

red notch
#

@vernal acorn we can totally do ##2178 in commands, but there are a few things that need to happen first

proud radishBOT
red notch
#

non-exhaustive list:

  • better support different command types, at present the extension is designed to support the least possible denominator of all types
  • improve support for dynamic registration and deregistration and support saving dynamic state to disk (maybe not directly, but a save-restore API users could call into), otherwise adding and removing features gradually is gonna get really fucking annoying
  • (also applies to slashcommands) make checks asynchronous and ideally support deferring early, currently checks will block you from deferring
  • remodel getting entities to getting 'promises' of entities that you can materialize later - or use skeleton entities (currently, the argument converters will block until they have all entities, so ie taking a params DiscordMessage[] is a very easy way to cause your command to time out
#

re the latter; it would also make life easier to not throw an error and cancel the entire command if a single input couldn't be fetched even though we may not even need to fetch it

#

if i want to, say, store a message for later linking and the bot isnt in that guild, it'll currently throw even though it doesnt actually need the message object

#

cc also @sharp fern

sharp fern
#

is there no possibility to have automatic deferal when a command is matched?

#

or does that only happen just prior of command execution

#

(wrt the last point)

#

okay I completely skimmed the third point

#

I think deferal should be moved to the earliest possible point (when the command is resolved, and known to defer, or if deferring is the default, as how the implementation works in Remora)

#

I'm not sure I understand the premise behind point number two however

#

As for point one... I had a long-winded thing written up but it devolves down to "I really don't like the notion of processors and feel that this discimination of command types has added tremendous overhead to doing anything with the commands library because of relying on fragile implementation details of inter-connected parts"

red notch
red notch
sharp fern
#

Good

red notch
#

have each event handler define a pipeline either through well-defined services or through dataflow pipelining

#

try to abstract each component

sharp fern
#

I mean ultimately it just comes down to "is this command an interaction" with maybe slightly different parameter ordering depending on if it's slash or a context menu

red notch
#

obv slashcommands-specific stuff gets to live in a slashcommands folder but generally we can loosen them a little

sharp fern
#

Pipelines are cool however

red notch
#

like, choice providers could just be generically built and we could support enums in text commands that way

#

the main problem is obviously that this is going to be a 20k diff lmao

sharp fern
#

A small cost

red notch
#

aint nobody got time

#

(im lying, i have the whole of february off uni)

#

(also i have three weeks of break from mid-december to early january)

red notch
#

@sharp fern / @vernal acorn ##2179 (get pinged)

proud radishBOT
sharp fern
red notch
#

niin

sharp fern
#

I can help with this

image.png
#

Looks good

red notch
#

i will slowly fill out each sub-section

#

feel free to help out with that if you have design in mind

sharp fern
#

I mean I can copy what I did for remora design wise

#

I think Lunar's already got most of that working down

#

namely just an issue of making the surface api better

red notch
#

yee

sharp fern
#

app.UseCommands().MapCommand("name" (string param1, int param2) => { /* do thing */ } ) is effectively what we're looking for right

red notch
#

yea

#

ideally we support space-separated subcommands in there too

#

MapCommand("top-level subcommand", (TextCommandContext context, string param1, int param2) => ...);

sharp fern
#

That's a bit more complicated but not impossible

red notch
#

i did put it into the second section specifically because i think it could be made a lot easier by tree changes

sharp fern
#

if the tree is mutable yes

#

it was problematic in Remora because the tree is immutable

#

so everything has to be done at registration and I really need to refactor it

#

You learn from your mistakes

#

it's easier (and probably better) to represent all commands with builders like CNext did, and then build that into a tree

#

in my Remora PR I basically had the pre-made tree, and then did some real fuckshit by matching commands and injecting nodes

red notch
sharp fern
#

Fair enough

#

just means that commands will have to be done during service collection building (wheras with minimal apis you can register new endpoints at any point)

#

which is still fine i think

red notch
#

(eventually i want to support changing the tree at runtime, but in a very specific, well-defined set of circumstances and requirements)

#

yeah thats fine

sharp fern
#

lgtm

#

back to dealing with fp16 fuckery until then
-# Damn the entirety of the IEE754 spec

red notch
#

@vast latch do you remember our conversations about the functions emitted for invoking commands

#

i'm currently on recreating the builders, and... yeah when

vast latch
red notch
#

i'm personally also mildly annoyed at all the comments in that code, they ironically make it less readable to me when

#

stop drawing my attention with trivial knowledge

vast latch
#

my code didn't do anything fancy here, my only advice is to declare a closure type for the state it may need (i.e. a delegate to create the command module) and offload as much as possible into c# helpers called from IL

red notch
#

i know what ldelem.ref does tyvm

red notch
vast latch
#

the delegate i meant here was something like what ActivatorUtilities provides

#

so it can be resolved with DI without actually being registered to the service provider

#

obviously doesn't apply to static functions, but i didn't support that

#

so my emitted code was essentially:

  • call helper to create module
  • load every argument from the array with cast
  • call command function
  • maybe wrap in VT
red notch
#

that way they can have actual lifetimes

#

unlike Commands which only supports faux transient, or CNext/SC which bleed root-scope-resolved commands when

vast latch
#

... i wonder if there is a practical reason to make them not transient? like you can still resolve them from a scope that way and you're not gonna resolve them twice in the same scope

#

and singletons seem... weird to me

red notch
#

singleton was compelling enough for CN/SC to support them

#

for the case of "i want it static but i still want services" i suppose

vast latch
#

tbf singleton would be easy to allow even without registration

#

just scoped gets funky

red notch
#

scoped also has a raison d'être, arguably, now that dsharpplus supports scoped more

vast latch
#

yeah but would you notice it's scoped vs transient

#

unless you manually resolve the module within a command

red notch
#

uh, yeah, if you have another thing in the same type activated by the same scope

#

event handlers (pre-execution hooks) come to mind

vast latch
#

i guess those would be the same scope but it still seems strange to resolve a command module from there

red notch
#

it does

#

but i don't think i hate it enough to not support it

vast latch
#

either way, adding them to DI is probably good too, just seems overkill to me

red notch
#

i wonder how big this PR is going to end up being

#

probably very

red notch
#

@vernal acorn about getting node builders from types/methodinfos

#

i have an issue

#

two issues

#

they're multiplying

#

first issue is that a MethodInfo with its own CommandAttribute (so, not just an overload of another) creates both an executable node and an overload node

#

and i'm really not sure how the API for that should look

#

FromMethodInfo() returning a CommandNodeBuilder with one pre-populated child?

#

that you then have to address

vernal acorn
#

it creates an overload node bcs there could be a second command with the name?

red notch
#

sort of yeah

#

an executable node is a node that can be executed without consuming more tokens of text for the command name

#

an overload node is an overload of an executable node, and for simplicity' sake executable nodes always have at least one overload representing them

#

that's where the actual execution info; so checks, execution delegate, context type etc are

vernal acorn
#

so smth like this?

red notch
#

no

#

wait

#

not nearly as neat but w/e

vernal acorn
#

i can recommend excalidraw when

red notch
#

a branch node is just a branch in the tree
an executable node can be directly executed without consuming more tokens from the stream
an overload is... an overload

vernal acorn
#

so branch is for subcommands etc

red notch
#

i'll add tree walker types that handle the job of scanning under certain requirements

#

so for example a slashcommands walker would treat executable nodes with subcommands (which is valid) as branch nodes

#

because that modeling isnt supported for slashcommands

vernal acorn
red notch
#

the current shape would be builder.Overloads[0] but that'll get very unwieldy very fast if we have multiple overloads

#

gonna add some sort of looking for parameter types

#

(likewise, subcommands are going to get name lookup)

vernal acorn
#

sorry im to tired to think..

red notch
#

:P

#

i'll add a second problem for when you're more awake

#

whitespace in command names

vernal acorn
#

i set a reminder for tmrw 2000

vernal acorn
red notch
#

allowed in context menus, violently breaks text commands

#

how do we handle

#

do we encode it with special metadata for context menus or

vernal acorn
#

didnt we have already a fixer for naming thinks?

red notch
#

undecidable problem

#

what exactly is a [SlashCommandType(MessageContext)] async Task Command(CommandContext, ...);

#

it's a valid text command

#

where spaces aren't allowed

#

but it's also a valid message context command, where they are allowed

vernal acorn
#

and its invalid for slashies when

red notch
vernal acorn
#

making generic things for stuff that has such specific rules sucks

red notch
#

i mean, we have a lowercased name that we can use for text commands

#

i would kinda like to not have to nest classes eight layers deep, personally

#

but we can also do like [Command("reminder", "add")] to indicate command nesting

sharp fern
#

sounds like that'd muddy already questionable handling of command trees

#

Well, depends on what metadata looks like

#

if you hide away type information it's fine

red notch
#

what does "that" refer to

sharp fern
#

allowing nesting via an array of names in the command attribute

red notch
#

how would that muddy anything?

sharp fern
red notch
#

that does not enlighten the point

sharp fern
#

I'll just go look at the code myself

#

There are several issues with the approach, but the weight of my concerns is heavily dependent on how abstract command trees actually are

red notch
#

approximately "now" would be a good time to go and discuss them, before i start building on them

red notch
#

narrator: they did not, in fact, elaborate "approximately now"

fading jackalBOT
#

@vernal acorn

<t:1743631138:R> you wanted to be reminded:

continue here

Id: 7
red notch
red notch
#

@sharp fern another reminder

sharp fern
red notch
#

this is kinda-sorta blocking me from progressing, i would like an elaboration

sharp fern
#

Well disregard

red notch
#

no

#

i aint writing ten thousand lines on top of that just to then rip it up

sharp fern
#

Well I don't have input on the matter because I went to go look at command code and promptly decided I no longer wanted to look at command code

My concern about attributes was the slight inconsistency in how those attributes are applied (anything applied to the command will be specific to that command even if two commands share the same group, and something else that probably mattered)

#

Applying attributes to all commands in the same synthetic group also sounds like a recipe for people to footgun

red notch
#

to 1. intentional, we want true overloading rather than what cnext did

red notch
sharp fern
#

I don't think either is amazing (synthetic groups being woo woo library magic), but that's also because I think nesting commands more than one layer deep begins to be awful UX to begin with ioa (I think slash commands technically allow 2 laters? at which point I ~guess~ complaining about nesting is fine, if you have a lot of commands to sift through, but partial exists, guh)

#

-# that being said, I'm not here to bikeshed, hence, disregard

red notch
#

@vast latch i summon the greater minds

#

so user permissions are easy enough to handle, each node requires its own permissions plus all predecessors'

vast latch
#

yeah, makes sense

red notch
#

it's rather absurd to have a command a b where b requires less privileges than is required for a

#

but what about bot permissions

vast latch
#

i'd expect bot permissions to work the same way, even though it clearly isn't necessary

red notch
#

it's perfectly conceivable that a b requires less bot permissions to execute than a

#

but its also kinda weird ux otherwise

vast latch
#

that is true, however

  • the dev will likely expect this to mirror user perms
  • it is easier to understand the behavior as a user of the bot later on
red notch
#

yeahh

vast latch
#

so i'd say inherit parent node permissions for that too

red notch
#

reasonable

vast latch
#

if you really want to have an "exclusive" mode, i guess an extra parameter on the bot permission attribute?

#

though i doubt that would ever be used

red notch
#

yeah that sounds like more effort than it's worth without user demand

vast latch
#

yeah

red notch
#

currently thinking about the command registration API

#

thinking of like cs builder.AddCommands() .Add<Type>() .Add<OtherType>() .Add<GuildSpecific>(id) .Finish();

#

(you can resume adding commands at any point)

#

maybe calling it ConfigureCommands would be more reasonable if it's also to expose the builders for editing

#

anyhow, my plans for assembly scanning are to have a ScanIn(Assembly) method that requires no manual registration of commands and checks whether you already tried that assembly

#

if youre loading the same assembly twice through different ALCs honestly just fuck you when

#

at that point no god in heaven can help you

red notch
#

cc @vernal acorn opinions?

sharp fern
red notch
vernal acorn
red notch
#

discordclientbuilder

#

analogous methods for IServiceCollection as per usual

vernal acorn
red notch
vernal acorn
#

lgtm

red notch
#

!test akiraveliara

elfin oarBOT
red notch
#

!test "#1 System.IO.Stream hater"

elfin oarBOT
red notch
#

!test akiraveliraa

elfin oarBOT
red notch
#

!test akiraveliara

elfin oarBOT
red notch
#

!test akiraveliraa

elfin oarBOT
red notch
#

!test "#1 System.IO.Stream hater"

elfin oarBOT
red notch
#

!test #1system.io.streamhater

elfin oarBOT
red notch
#

!test akiraveliara

elfin oarBOT
red notch
#

!test akiraveliraa

elfin oarBOT
red notch
#

!test "#1 System.IO.Stream hater"

elfin oarBOT
red notch
#

!test "#1 System.IO.Stream hater 🌈"

elfin oarBOT
red notch
#

!test akiraveliara

elfin oarBOT
red notch
#

!test akiraveliraa

elfin oarBOT
red notch
#

hyvä

#

oh that was meant to go in #dev-text