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

Multistream support #89

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

holy-jesus
Copy link
Contributor

Tested on Windows and Linux, everything works.

User can set text source in any script or even in both, but when set in both may sometimes blink between two states, very rarely and not noticeably.

enforce_id=False because i had some problems with rcon connection, probably because of the two connection at the same time, found solution here.

Copy link
Contributor

@acuifex acuifex left a comment

Choose a reason for hiding this comment

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

I think figuring out some inter plugin communication would be better, or at least merging the two scripts into one. Also i think some abstractions wouldn't hurt, something like VotePlatformInterface that youtube and twitch implement, because right now the code is quite repetitive (and also bad but that's my fault lol).


// New vote
if (args.ArgC() >= 3) {
g_arriVotes[atoi(args[2])] = g_arriVotes[atoi(args[2])] + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Memory corruption bug here. g_arriVotes is only 4 values long

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that when was testing, but forgot 💀

);

}
CON_COMMAND(chaos_vote_internal_set, "used by an external client. sets current votes")
CON_COMMAND(chaos_vote_internal_update, "used by an external client. updates current votes")
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason i was setting instead of updating votes is so we don't have to maintain two separate states. I think doing it this way is error prone, leads to worse readability, and increases the amount of rcon calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You right, but what is alternatives? Set youtube and twitch votes in two separate arrays and then sum them when choosing effect? Or you can recommend something better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually wanted to mention this option, but completely forgot when writing lol. Something like a map of name:votes and you'd call chaos_vote_internal_set youtube 42 1 2 3 4

@holy-jesus
Copy link
Contributor Author

It seems to me that interaction between plugins would complicate things too much. I think that best solution to combine two integrations into one file, no repetitive code, less scripts user need to setup, and we can store votes from twitch and youtube in single script, which allows to not change anything in hl2_player.cpp. What do you think?

@acuifex
Copy link
Contributor

acuifex commented Feb 27, 2024

I've checked and it's actually easy to communicate between plugins. OBS shares python context between scripts.
Here's an example:
https://gist.github.com/acuifex/bf9092b2bdbc1894e1a0b4171c7b3f8f

So we'd implement platform-specific stuff in the scripts, and put the rest in libraries. This would require a bit of hacking with libraries so only one of the scripts runs libs functions.

@Pinsplash
Copy link
Owner

does this still have the problems acuifex pointed out?

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.

3 participants