Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more basic Twitch conditions, simplify some code and make it more consistent #902

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

Destroy666x
Copy link
Contributor

@Destroy666x Destroy666x commented Oct 23, 2023

This PR:

  • adds all basic Twitch EventSub conditions, except ones marked as beta, incompatible with websockets and one not too useful user one
  • allows multiple token permissions per condition
  • refactors code:
  1. Simplifies generic Twitch event related functions for much less code. More advanced logic can go into new functions.
  2. Separates basic events (which are always nice to have unchanged to just bump versions and quickly test in case of changes), advanced logic events and polling in a clearer and more consistent way.
  3. Changes the condition numbering for more leeway. Basic events get +100 just in case, I can imagine 10 treshold being easily crossed with related advanced logic conditions at some point, this is more than enough, but better safe than sorry to keep things organized IMO. And polling starts at much higher number for the same reason.

I tested a bunch of conditions, I'll test some more today or tommorow live, the rest will need to be checked by people as I can't really access some features too conveniently.

Once this gets merged, I'll start working on a bunch more ID/regex matching conditions (e.g. specific points rewards or prediction/poll option winning)

@WarmUpTill
Copy link
Owner

Looks great!

Just found one minor issue:
The ConditionIsSupportedByToken() function seems to have returned true if either of the token options is set.
I adjusted to check if all options are set.

Also can you maybe also shorten the commit message of c772b40 or would you mind if I did so?

Thanks again for your contributions!

@Destroy666x
Copy link
Contributor Author

Destroy666x commented Oct 23, 2023

Feel free to shorten the commit if needed (why just out of curiosity?)

@WarmUpTill
Copy link
Owner

WarmUpTill commented Oct 23, 2023

Just want to adhere to the general git commit guidelines to avoid the commits being cut off with the "...":
grafik
I will adjust this once the PR is ready to be merged.

I had another look at the event handling:
I think the raid events (possibly more?) might need special treatment as here the broadcaster_user_id filed is replaced with to_broadcaster_user_id and from_broadcaster_user_id respectively.
So with the current implementation these conditions can never be true.

@Destroy666x
Copy link
Contributor Author

Destroy666x commented Oct 23, 2023

Not sure what you mean, they already have special handling.

Ah, I see, the condition check also had broadcaster ID verification. I'll add a string param for that like in the other function once I come back home

@Destroy666x
Copy link
Contributor Author

Fixed, quick scrolling didn't show any exceptions other than raid for this field.

@Destroy666x Destroy666x force-pushed the AddMoreTwitchConditions branch from d1d647b to 6d37d5c Compare October 23, 2023 21:28
WarmUpTill and others added 2 commits October 25, 2023 14:45
@WarmUpTill WarmUpTill force-pushed the AddMoreTwitchConditions branch from 6d37d5c to 95ba17d Compare October 25, 2023 12:46
@WarmUpTill WarmUpTill merged commit 890792c into WarmUpTill:master Oct 25, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants