Conversation
|
okii, i initially thought of making another issue for this, but i think it actually applies more to my commands (maybe not though since the unauthorized call is actually kinda helpful for ban/unban), so another issue dealing with command failuresso i feel like currently with commands, i know it logs stuff, but there's not really anything in the chat that says something went wrong except an absence of the sucessful command message, maybe that is good enough but i feel like there are a few "expected" error cases that are nice to handle (for example, giving someone vip/unvip can fail for the 3 expected cases of not enough permissions, dont have affiliate so not able to, or the target already has vip/doesnt) |
|
The way I send messages to the chat for errors sent from the twitch API is through The reason why On top of all that yeah there's a bunch of duplication that I don't really like, didn't think of it too much when originally creating it and the mess just expanded over time. I'm probably going to redo a lot of it in the near future to make implementing a Twitch API call much more sane. Usually I try to match Twitch as much as possible in terms of message responses, but I don't have a problem with using different messages if it's seems better to do so. Thanks for the PR! |
|
okii, ill add tests to the commands file (and probably will just delete the parse_commands file) for what the twitch messages are, honestly i like had no idea how to find them, i like legit didnt think of just going into my own twitch chat and just typing in the commands sldkfjdkslfj that makes better sense, ill update mine to fit those better about the error text or just general error handling for commands, would that be a good thing to open a seperate issue to discuss on? ig there's probably some weird wrapper function u could do to dedupe the commands parsing but idk enough about rust enums to know if thats actually like remotely easy to do or not, |
|
I think for now in terms of error handling commands you can just send a |
|
(idk how to reference it so twitch/mod.rs at 142) if let Some(command) = message.strip_prefix('/') {
if let Err(err) = handle_command_message(&context, &tx, command).await {
error!("Failed to handle Twitch message command from terminal: {err}");
}
}i did this in my own branch but im not sure if its good so would it make sense to have this also do a tx send or to just error handle seperately inside the handle_command_message function |
|
I think for now it's fine to have the error log followed by tx.send |
|
okii ty :3 |
|
i believe this should be everything needed (other than dedupes and such), feel free to make ur own changes or ask anything else that might be required for this, |
|
Very cool. I'll do a full review within the next few days, but a quick skim over all the changes look good to me as of now. |
|
@kaizoplant are you going to do more changes or is it ready for review as of now? Just making sure such that my review doesn't go stale with planned changes. Thanks! |
|
(okay apparently idk how git interacts with emails) yeah i think im ready for a review now, thanks :3 |
Xithrius
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for the PR!
this pr adds the commands unban, raid, unraid, followers, followersoff, slow, slowoff, subscribers, subscribersoff, emoteonly, emoteonlyoff, vip, unvip, mod, and unmod
i also used these 2 in my own branch so it includes two commands that are not native in twitch, those being title and category to update stream information, but those can probably be removed and maybe have an issue to discuss them/a separate pr if it doesnt make sense to have them here
uh i think uh its also still not ready rn bcs of the following
testing
okay this is something i didnt notice in the original branch, but im not sure where are the tests for TwitchCommand::from_str are supposed to be, as they seem to be both in tests/parse_commands.rs and twitch/commands.rs, id assume twitch/commands.rs since it seems to have been more recently created? but im not sure,
query types and command parsing
so, I've followed the format for query types as is used for TimeoutQuery, but there's some duplication between my types, since for example vip and mod would use the same type as each other since they just require broadcaster and user information, im not sure if its just fine as is or if it should have just one differently named type,
as for command parsing, its very duplicated, with multiple of them essentially doing the same thing, and im also unsure of if i should be passing it that way in the first place, especially for commands that only take one extra argument
command responses
i have kinda put placeholder messages when a command happens, idk if it should be matching how twitch would respond or not, or if you just would have better messages to send there