Youtube voting #1
Conversation
.gitignore
Outdated
| @@ -28,6 +28,8 @@ dist/bundle.js | |||
|
|
|||
| # Visual Studio 2015/2017 cache/options directory | |||
| .vs/ | |||
| # Visual Studio Code cache/options directory | |||
There was a problem hiding this comment.
I'm not sure if we want this in the gitignore. If VS Code is actually used as one of the main editors, configuration should be synced. You may want to add this to the exclude file in the .git directory, which isn't synced (https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files#excluding-local-files-without-creating-a-gitignore-file), or actually integrate the VS Code config into the project. The latter would need to be discussed with pongo.
There was a problem hiding this comment.
you're right, it's not necessary to to have this here. Thanks for the advice
| @@ -75,6 +75,7 @@ public bool RequireBool(string key) | |||
|
|
|||
| try | |||
| { | |||
|
|
|||
There was a problem hiding this comment.
The main changes in this file seem to be newlines. Any reason for those? Is that according to some c# formatting guide? If yes, I'm happy to include them.
There was a problem hiding this comment.
I think that happened when I tried to resolve a conflict. When I forked your branch this wasn't fixed, so in my original pr I included a insecure quick fix. now that you've taken care of that I didn't have to have touched that file at all.
| @@ -5,5 +5,6 @@ enum EVotingReceiver | |||
| { | |||
| Twitch, | |||
| YouTube, | |||
There was a problem hiding this comment.
If this proxy is added, we might want to rename the current (my) implementation to YouTubeV3, but that's not important right now.
| buf[index] = (byte)val; | ||
| return buf; | ||
| } | ||
| public static byte[] concat(byte[] one, byte[] two) |
There was a problem hiding this comment.
I think we should use LINQ like described in this answer instead of creating a custom implementation https://stackoverflow.com/a/59262/10315665?
There was a problem hiding this comment.
VS Code shows me an error when I try to use the Concat method. Maybe I'll just use the overload which takes a list of byte[].
| } | ||
| return ret; | ||
| } | ||
| public static byte[] tp(long a, long b, byte[] ary) |
There was a problem hiding this comment.
Some more descriptive naming would be nice. Like, I have no idea what tp, or rs is.
There was a problem hiding this comment.
I have no idea either. That's why the class is called Magic. As I told you on Discord I copied that "algorithm" from a python library which had the same undescriptive names.
|
|
||
| private string build(long[] ts) | ||
| { | ||
| var b1 = new byte[] { 0x08, 0x00 }; |
There was a problem hiding this comment.
Technically, doing byte[] b1 = { 0x08, 0x00 }; would save us the new keyword. But I'm not that much of a c# expert to tell what's better.
But I can tell you I've no idea what's going on, lol. I'm assuming this is just copied from somewhere?
There was a problem hiding this comment.
Should I make a second pull request after all those change or is there a way to update the current one?
| JsonObject responseData = res["continuationContents"]!["liveChatContinuation"]!.AsObject(); | ||
| var continuations = responseData!["continuations"]!.AsArray()![0]!.AsObject(); | ||
| JsonNode? continuationData; | ||
| if (continuations.TryGetPropertyValue("invalidationContinuationData", out continuationData)) |
There was a problem hiding this comment.
One shouldn't use the null-forgiving operator. Since it's technically an arbitrary JSON object, all fields could potentially be null. So you should probably use the ? (null conditional), and check afterwords if the value exists. Maybe you could even chain the call? Like:
continuations.TryGetPropertyValue("invalidationContinuationData")
.?["continuation"]
.?GetValue<string>();But I'm not sure, since the API uses that weird out thing (which is of course not your fault lol).
There was a problem hiding this comment.
Oh, and btw. the upstream is using some other JSON parsing thing. Although, I haven't been able to merge that yet, since the c++ part keeps crashing on me :(. But maybe we could use the same library? Looks pretty clean: gta-chaos-mod@b2c9732#diff-19156a6e6f3132fe9e2de63a68a651114269e429e04a09898a88d5fb8af64bdcL1-R171.
|
Your coding style, using early returns instead of nested ifs, is pretty nice btw :). Easier to read this way. |
Implemented the voting receiver that uses the Youtube API v1, so it doesn't need an API key.
Requirements for the voting.ini file:
VotingReceiver=YoutubeApiV1,YoutubeChannelId,LiveStreamVideoIdand the usual stuff.I think now we can talk to pongo and test this with a more "spammy" chat than the lofigirl stream that I've been using so far.
Also, I dont think the ChaosConfigApp supports the youtube options. Maybe I'll tackle this next