Remora / Remora.Discord

A data-oriented C# Discord library, focused on high-performance concurrency and robust design.
GNU Lesser General Public License v3.0
243 stars 46 forks source link

Fall back to default command trees when the command is not found. #258

Closed yretenai closed 1 year ago

yretenai commented 1 year ago

Description

It was deprecated in this commit: https://github.com/Remora/Remora.Discord/commit/c0477dfcff9b0e7bb7d0dff236ca6fb27276e4d1 without explanation on why it was removed.

Though it's current state in 2202.49 is broken since InteractionResponder.TryExecuteCommand casts a Result into a Result which will set the success state to true (rather than preserving the existing result state), even if this behavior was not what was observed it will retry with the same treeName -- this will make it successfully fail with a CommandNotFound result and the default tree will never be evaluated (see below.) I initially was going to report this as a bug but that's when I found the removal commit. This exact bug is happening all over the project. Every time a Result\<T> is cast to Result, it will be successfull

Screenshot_20221130_014405

Why This is Needed

I am designing a system where certain command groups are opt-in.

Alternatives Considered

Specifying the command groups used in the global tree in the named trees (i.e. merging the two trees,) results in Discord showing the commands twice in the command list. (See attached image below)

Screenshot_20221130_012736

Rejecting the command on the "command level" still shows it in the command list and will leave a dangling interaction or response message behind.

Additional Details

Ultimately I ended up writing my own Interaction Responder which runs through a list of trees rather than just one + default.

VelvetToroyashi commented 1 year ago

There was a reason, actually, which was briefly mentioned in the PR that commit was linked to.

The change greatly simplified command execution flow. image

VelvetToroyashi commented 1 year ago

(Actually, I just realized this image is technically inaccurate, oops, but the main gist is still correct.)

Nihlus commented 1 year ago

Is the content of the issue valid, since you closed it? The body of the described problems suggest more than one issue, but it's unclear which (if any) are moot or resolved.