[WIP] Feature command rewrite #862
Conversation
… there Remove dead code from Archi
ArchiSteamFarm/Bot.cs
Outdated
| await HandleMessage(notification.chat_group_id, notification.chat_id, notification.steamid_sender, message).ConfigureAwait(false); | ||
|
|
||
| string response = await Commands.Parse(this, notification.steamid_sender, message).ConfigureAwait(false); | ||
| if(response == null) { |
There was a problem hiding this comment.
Missing space + use null or empty for strings (nearly always, unless it matters for you whether you have null or empty).
ArchiSteamFarm/Bot.cs
Outdated
| @@ -1982,7 +1957,12 @@ internal sealed class Bot : IDisposable { | |||
| return; | |||
| } | |||
|
|
|||
| await HandleMessage(notification.steamid_friend, message).ConfigureAwait(false); | |||
| string response = await Commands.Parse(this, notification.steamid_friend, message).ConfigureAwait(false); | |||
| if(response == null) { | |||
There was a problem hiding this comment.
Same here. And you missed return too, so it'll crash instead 😉.
JustArchi
added
the
✨ Enhancement
ArchiSteamFarm/Bot.cs
Outdated
| await HandleMessage(notification.steamid_friend, message).ConfigureAwait(false); | ||
| string response = await Commands.Parse(this, notification.steamid_friend, message).ConfigureAwait(false); | ||
| if(response == null) { | ||
| ArchiLogger.LogNullError(nameof(response)); |
There was a problem hiding this comment.
Do not blindly copy paste, LogNullError() should be used ONLY for UNEXPECTED errors, null response is entirely expected when the user doesn't have permission to interact with the bot and therefore generates no response.
|
I just noticed that thanks to the dicitionary things like (Code coming in next commit) |
…s where we have a lambda already, Fix formatting
ArchiSteamFarm/Commands.cs
Outdated
| internal sealed class Commands { | ||
| //Because this dictionary maps to async functions we have to make sync functions async with lambdas; e.g. VERSION | ||
| //Because for some commands there exist versions with AND without parameters we have to use lambdas to adapt function-interfaces sometimes; e.g. STOP | ||
| private static readonly Dictionary<(string Command, byte ArgumentCount), Func<Bot, ulong, string[], Task<string>>> CommandDictionary = new Dictionary<(string Command, byte arguments), Func<Bot, ulong, string[], Task<string>>>() { |
There was a problem hiding this comment.
This is wrong on so many levels. Why do you even put args number there, didn't I just tell you to make static version for each command and in that static version do any kind of arguments parsing? Why do you put lambdas here instead of JUST a pointer to a function that you will await/execute in Parse() function? You didn't just kill old working functionality, but you did it for absolutely no reason - this is unacceptable.
…ent-parsing inside them
ArchiSteamFarm/Commands.cs
Outdated
| return string.Join(Environment.NewLine, validResults.Select(result => result.Response).Union(extraResponse.ToEnumerable())); | ||
| } | ||
|
|
||
| private static string GetSingleBotStatusResponse(Bot bot, ulong steamID) { |
There was a problem hiding this comment.
Name it the same as parent function, so ResponseStatus. This one accepts less arguments anyway.
…viously forgotten
…y in one occurence
|
Take a break for a week or so (I expect hotfixes in master regarding #810) and then rebase against master, thanks. |
|
seems legit. v3.3 when all hotfixes are completed? |
|
So definitely NOT 3.3.0.0 😉 |
Implements #861 upon completion by rewriting ASFs internal handling of commands and within this change cleans up the huge file Bot.cs already is.
Any advice welcome. 👍