feat: Live Emote Updates for 7TV #4090

Nerixyz · 2022-10-30T08:42:47Z

Pull request checklist:

CHANGELOG.md was updated, if applicable

Description

This PR adds live emote updates (EventApi) for 7TV (see previous PR #4087 and discussion #4038). It's rather large, since it adds building blocks (mainly for adding the messages) for other emote providers.

Tests won't pass until Chatterino/twitch-pubsub-server-test#30 is merged and a new version of the tests is released.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 41. Check the log or trigger a new build to see more.

src/messages/MessageBuilder.cpp

src/providers/seventv/SeventvEventApi.cpp

src/providers/seventv/eventapi/SeventvEventApiClient.cpp

src/providers/seventv/eventapi/SeventvEventApiDispatch.cpp

github-actions

clang-tidy made some suggestions

src/providers/seventv/eventapi/SeventvEventApiDispatch.hpp

src/providers/seventv/eventapi/SeventvEventApiMessage.hpp

src/providers/seventv/eventapi/SeventvEventApiSubscription.hpp

src/providers/twitch/TwitchIrcServer.cpp

tests/src/SeventvEventApi.cpp

github-actions

clang-tidy made some suggestions

src/providers/twitch/TwitchChannel.cpp

github-actions

clang-tidy made some suggestions

src/providers/twitch/TwitchChannel.cpp

github-actions

clang-tidy made some suggestions

src/providers/twitch/TwitchChannel.cpp

pajlada · 2022-10-30T12:10:29Z

.github/workflows/test.yml

@@ -6,7 +6,7 @@ on:
  workflow_dispatch:

 env:
-  TWITCH_PUBSUB_SERVER_IMAGE: ghcr.io/chatterino/twitch-pubsub-server-test:v1.0.4
+  TWITCH_PUBSUB_SERVER_IMAGE: ghcr.io/chatterino/twitch-pubsub-server-test:master


Reminder for myself to change to v1.0.5 once we've tested this PR and pushed a new release for the pubsub server test

src/providers/twitch/TwitchChannel.cpp

src/providers/twitch/TwitchChannel.hpp

src/providers/seventv/SeventvEmotes.hpp

src/providers/seventv/SeventvEmotes.cpp

src/Application.cpp

src/providers/seventv/SeventvEventApi.hpp

pajlada · 2022-11-06T12:33:42Z

src/providers/seventv/SeventvEventApi.hpp

+
+    std::unordered_set<QString> subscribedEmoteSets_;
+    std::unordered_set<QString> subscribedUsers_;
+    std::chrono::milliseconds heartbeatInterval_;


hearbeatInterval_ can be made const if we end up keeping it. If it's removed and replaced with a defaultHeartbeatInterval it can just be set in the constructor of the SeventvEventApiClient instead with a magic value

github-actions

clang-tidy made some suggestions

src/messages/MessageBuilder.cpp

src/providers/seventv/SeventvEventAPI.cpp

src/providers/twitch/TwitchChannel.cpp

tests/src/SeventvEventAPI.cpp

zneix · 2022-11-10T11:15:15Z

Make the whole websocket completely optional, would be cool to be able to completely not include it at the compilation process to not bloat the application...

pajlada · 2022-11-10T11:32:03Z

Make the whole websocket completely optional, would be cool to be able to completely not include it at the compilation process to not bloat the application...

If the setting is disabled, the class is never instantiated so run-time this is a non-issue.

We haven't done compile-time feature flags before, the closest we've gotten to it is enabling/disabling QtKeychain but the file is still there.
I'm happy to explore the idea of compile-time feature flags as it's not a foreign concept but I'd like to hear more justification for it.

Nerixyz · 2022-11-10T12:41:53Z

to not bloat the application...

You'd still include websocketpp for PubSub, so you won't save much space:

feat/seventv-liveupdates@9ef333d vs master@3303cdc, Qt 5.12.12

Windows 17.627kb -> 17.698kb, +71.0kb, +0.40%
macOS 15.633kb -> 15.734kb, +101.0kb, +0.65%
AppImage 34.287kb -> 34.423kb, +136.0kb, +0.40%

This would also mean that CI will need to run 4 additional builds, or 8 for all Qt versions.

pajlada

Just some teeny tiny things 👍

src/messages/Emote.hpp

src/providers/seventv/eventapi/SeventvEventAPIClient.hpp

aka EventApi

This removes the copy constructor which we need in the tests.

github-actions

clang-tidy made some suggestions

src/providers/twitch/TwitchChannel.cpp

github-actions bot reviewed Oct 30, 2022

View reviewed changes

Nerixyz mentioned this pull request Oct 30, 2022

feat: Add 7TV EventApi Tests Chatterino/twitch-pubsub-server-test#30

Merged

github-actions bot reviewed Oct 30, 2022

View reviewed changes

src/providers/twitch/TwitchChannel.cpp Outdated Show resolved Hide resolved

Nerixyz force-pushed the feat/seventv-liveupdates branch from 4c0d575 to 7e56a76 Compare October 30, 2022 15:57

AnatoleAM approved these changes Oct 30, 2022

View reviewed changes

Nerixyz force-pushed the feat/seventv-liveupdates branch from 7e56a76 to 5aa5832 Compare October 31, 2022 14:18

github-actions bot reviewed Oct 31, 2022

View reviewed changes

src/providers/twitch/TwitchChannel.cpp Outdated Show resolved Hide resolved

b-ggs mentioned this pull request Nov 3, 2022

Enabling an emote with multiple versions results in an older version being loaded in SevenTV/EventAPI#81

Closed

Nerixyz force-pushed the feat/seventv-liveupdates branch from 5aa5832 to 6c312d5 Compare November 3, 2022 12:49

github-actions bot reviewed Nov 3, 2022

View reviewed changes

src/providers/twitch/TwitchChannel.cpp Outdated Show resolved Hide resolved

b-ggs mentioned this pull request Nov 3, 2022

Bug report: Enabling an emote with multiple versions results in an older version's emote data being loaded in SevenTV/API#127

Closed

pajlada reviewed Nov 6, 2022

View reviewed changes

pajlada requested changes Nov 6, 2022

View reviewed changes

Nerixyz force-pushed the feat/seventv-liveupdates branch 2 times, most recently from b6ea764 to 44bb728 Compare November 6, 2022 20:57

github-actions bot reviewed Nov 6, 2022

View reviewed changes

Nerixyz force-pushed the feat/seventv-liveupdates branch from 9ef333d to d6284a9 Compare November 11, 2022 19:39

pajlada requested changes Nov 12, 2022

View reviewed changes

src/messages/Emote.hpp Show resolved Hide resolved

src/providers/seventv/eventapi/SeventvEventAPIClient.hpp Outdated Show resolved Hide resolved

Nerixyz added 8 commits November 13, 2022 09:56

feat: seventv live updates

a523414

aka EventApi

fix: handle reconnect

047b3c0

fix: set user-agent

caa6b11

fix: update test definitions

0a329f2

chore: update changelog

bd55e0f

fix: whitespace

8f0c853

ci: use master image

77a1e76

fix: make functions const

c4eaed1

pajlada and others added 18 commits November 13, 2022 09:56

Uppercase API

1801eb6

Rename some more variables and functions

ec3cf4d

fix: clang-tidy warnings

3f4ea71

fix: minor fixes

f6ca35b

fix: funny bsd thing

ac8f24d

fix: require channel emotes for eventapi

71150e2

fix: suggestions

2dcc6b0

refactor: store last message in channel

a4e4575

fix: use connection index when updating a user

89c7577

docs: twitch channel docs

21c7152

fix: constification

2afbbc4

fix: filenames

f2c1783

revert: constification

30500e2

This removes the copy constructor which we need in the tests.

test: connection id

3ba89bd

fix: document Emote::baseName

cc77557

fix: make heartbeatInterval non-atomic

3ecb005

fix: casing

6572cfd

fix: wording

05e37e0

Nerixyz force-pushed the feat/seventv-liveupdates branch from d6284a9 to 05e37e0 Compare November 13, 2022 08:56

github-actions bot reviewed Nov 13, 2022

View reviewed changes

src/providers/twitch/TwitchChannel.cpp Show resolved Hide resolved

src/providers/twitch/TwitchChannel.cpp Show resolved Hide resolved

change twitch pubsub server image back to stable release, but v1.0.5

9221af2

pajlada approved these changes Nov 13, 2022

View reviewed changes

pajlada mentioned this pull request Nov 13, 2022

Document our use of 7TV's Event API Chatterino/wiki#252

Merged

pajlada merged commit 39f7d8a into Chatterino:master Nov 13, 2022

Nerixyz deleted the feat/seventv-liveupdates branch November 13, 2022 11:07

LosFarmosCTL mentioned this pull request Jan 31, 2023

Crashes all throughout the application when destroying channel objects on macOS #4347

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Live Emote Updates for 7TV #4090

feat: Live Emote Updates for 7TV #4090

Nerixyz commented Oct 30, 2022

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

pajlada Oct 30, 2022

pajlada Nov 6, 2022

github-actions bot left a comment

zneix commented Nov 10, 2022 •

edited

pajlada commented Nov 10, 2022

Nerixyz commented Nov 10, 2022

pajlada left a comment

github-actions bot left a comment

feat: Live Emote Updates for 7TV #4090

feat: Live Emote Updates for 7TV #4090

Conversation

Nerixyz commented Oct 30, 2022

Description

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

pajlada Oct 30, 2022

Choose a reason for hiding this comment

pajlada Nov 6, 2022

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

zneix commented Nov 10, 2022 • edited

pajlada commented Nov 10, 2022

Nerixyz commented Nov 10, 2022

pajlada left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

zneix commented Nov 10, 2022 •

edited