Skip to content

add further websocket commands#679

Merged
Xithrius merged 7 commits intoXithrius:mainfrom
kaizoplant:main
Jul 6, 2025
Merged

add further websocket commands#679
Xithrius merged 7 commits intoXithrius:mainfrom
kaizoplant:main

Conversation

@kaizoplant
Copy link
Contributor

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

@kaizoplant
Copy link
Contributor Author

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 failures

so 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)

@Xithrius
Copy link
Owner

Xithrius commented Jul 2, 2025

The way I send messages to the chat for errors sent from the twitch API is through tx.send(DataBuilder::twitch("placeholder message")).await?, and it doesn't appear much throughout the codebase, which is my bad for not really "standardizing" it. What I also noticed it that Twitch doesn't always send error text such as why something was a bad request. I had this scuffle when dealing with the broken timeout and ban commands because I'd just get a 400 without reason, when it should have told me that the payload was invalid (I had done a vector on the inner payload and not just a separate structure on top of it as a wrapper).

The reason why tests/parse_commands.rs exists for Twitch Command parsing is because I forgot it was a thing and started writing tests in twitch/commands.rs (came back to the repo after a while and forgot parts of the structure lol). Either way it should probably be in twitch/commands.rs since that's where the TwitchCommand enum is defined in the first place.

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!

@kaizoplant
Copy link
Contributor Author

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,

@Xithrius
Copy link
Owner

Xithrius commented Jul 2, 2025

I think for now in terms of error handling commands you can just send a DataBuilder::Twitch message that describes the error without having to reference Twitch's messages, since it's not really necessary at the moment. Maybe I can find a reference for all the different possible error messages, but that's probably for a later time.

@kaizoplant
Copy link
Contributor Author

kaizoplant commented Jul 2, 2025

(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

@Xithrius
Copy link
Owner

Xithrius commented Jul 2, 2025

I think for now it's fine to have the error log followed by tx.send

@kaizoplant
Copy link
Contributor Author

okii ty :3

@kaizoplant
Copy link
Contributor Author

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,

@Xithrius
Copy link
Owner

Xithrius commented Jul 2, 2025

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.

@Xithrius
Copy link
Owner

Xithrius commented Jul 5, 2025

@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!

@kaizoplant
Copy link
Contributor Author

(okay apparently idk how git interacts with emails) yeah i think im ready for a review now, thanks :3

Copy link
Owner

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for the PR!

@Xithrius Xithrius merged commit 2e3b6b6 into Xithrius:main Jul 6, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants