Allow commands to return a Task<RuntimeResult> #466
Conversation
|
What's the use-case for this? I'm not seeing how this is any better than throwing a custom exception and pattern-matching the type of the exception. |
|
This was discussed in the Dev Guild. |
|
does it look like im reading the dev guild?
…On Jan 6, 2017 14:52, "Khionu Sybiern" ***@***.***> wrote:
This was discussed in the Dev Guild.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#466 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AMff8p5H5m9kG8y2HIvhyNVX8M2P10dQks5rPpuFgaJpZM4Lcx-J>
.
|
|
TL;DR, custom errors, or passing any data from the commands. Good for any PostCommand Logic |
| @@ -249,7 +249,7 @@ private static bool IsValidModuleDefinition(TypeInfo typeInfo) | |||
| private static bool IsValidCommandDefinition(MethodInfo methodInfo) | |||
| { | |||
| return methodInfo.IsDefined(typeof(CommandAttribute)) && | |||
| methodInfo.ReturnType == typeof(Task) && | |||
| (methodInfo.ReturnType == typeof(Task) || methodInfo.ReturnType == typeof(Task<RuntimeResult>)) && | |||
There was a problem hiding this comment.
Since this PR (allegedly) still needs a discussion, I'm throwing this note in here: this check won't work entirely well when people want to return a Task<CustomRuntimeResult>.
Disregard this, I know where this decision is coming from now.
| break; | ||
| { | ||
| var task = _action(context, args, map); | ||
| var returnTask = task as Task<RuntimeResult>; |
FiniteReality
force-pushed
the
feature/command-returns
branch
from
ee1b557
to
180f517
Compare
| [DebuggerDisplay(@"{DebuggerDisplay,nq}")] | ||
| public class RuntimeResult : IResult | ||
| { | ||
| private RuntimeResult(CommandError? error, string reason) |
There was a problem hiding this comment.
Should be protected for allowing subclassed RuntimeResults.
There was a problem hiding this comment.
🙃 I'm sure I managed to subclass this when I tested it originally, good catch though.
| { | ||
| await Module.Service._cmdLogger.DebugAsync($"Executing {GetLogText(context)}").ConfigureAwait(false); | ||
| try | ||
| { | ||
| await _action(context, args, services).ConfigureAwait(false); | ||
| var task = _action(context, args, services); | ||
| if (task is Task<IResult> returnTask) |
There was a problem hiding this comment.
I might be crazy, but this doesn't seem like it actually works. A quick test with this code seems to print Nope every time.
It's possible to do if (task is Task<RuntimeResult> resultTask) instead, and not need the second check.
There was a problem hiding this comment.
It does work, but probably not for users using command builders. Whoops.
The reason why I needed to return Task is because of ModuleClassBuilder's Callback code: it returns a Task, and unfortuantely due to contravariance not working the way I wanted it to, it would never trip the condition.
This allows bot developers to centralize command result logic by
using result data whether the command as successful or not.
Example usage:
```csharp
var _result = await Commands.ExecuteAsync(context, argPos);
if (_result is RuntimeResult result)
{
await message.Channel.SendMessageAsync(result.Reason);
}
else if (!_result.IsSuccess)
{
// Previous error handling
}
```
The RuntimeResult class can be subclassed too, for example:
```csharp
var _result = await Commands.ExecuteAsync(context, argPos);
if (_result is MySubclassedResult result)
{
var builder = new EmbedBuilder();
for (var pair in result.Data)
{
builder.AddField(pair.Key, pair.Value, true);
}
await message.Channel.SendMessageAsync("", embed: builder);
}
else if (_result is RuntimeResult result)
{
await message.Channel.SendMessageAsync(result.Reason);
}
else if (!_result.IsSuccess)
{
// Previous error handling
}
```
It never really made sense to have it instantiable in the first place, frankly.
FiniteReality
force-pushed
the
feature/command-returns
branch
from
0d814ae
to
71a1adb
Compare
* Allow commands to return a Task<RuntimeResult>
This allows bot developers to centralize command result logic by
using result data whether the command as successful or not.
Example usage:
```csharp
var _result = await Commands.ExecuteAsync(context, argPos);
if (_result is RuntimeResult result)
{
await message.Channel.SendMessageAsync(result.Reason);
}
else if (!_result.IsSuccess)
{
// Previous error handling
}
```
The RuntimeResult class can be subclassed too, for example:
```csharp
var _result = await Commands.ExecuteAsync(context, argPos);
if (_result is MySubclassedResult result)
{
var builder = new EmbedBuilder();
for (var pair in result.Data)
{
builder.AddField(pair.Key, pair.Value, true);
}
await message.Channel.SendMessageAsync("", embed: builder);
}
else if (_result is RuntimeResult result)
{
await message.Channel.SendMessageAsync(result.Reason);
}
else if (!_result.IsSuccess)
{
// Previous error handling
}
```
* Make RuntimeResult's ctor protected
* Make RuntimeResult abstract
It never really made sense to have it instantiable in the first place,
frankly.
This allows bot developers to centralize command result logic by using result data whether the command was successful or not.
Example usage:
The RuntimeResult class can be subclassed too, for example: