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

Implement Personal Emotes #192

Merged
merged 13 commits into from
Mar 11, 2023
Merged

Conversation

Nerixyz
Copy link
Collaborator

@Nerixyz Nerixyz commented Feb 24, 2023

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

This PR implements personal emotes.

Known bugs/tasks:

  • The personal emote set is only fetched after typing a message. So you can't autocomplete emotes immediately.
  • Personal emotes only show up in the second message (same issue as for badges).
  • Personal emotes are not yet deleted after they haven't been used in a while (the images for the emotes however are deleted as always).
  • Personal emotes should show in the Subs tab instead of Channel.
  • Only approved personal emotes should be shown.
  • Add a setting to disable personal emotes

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/providers/seventv/SeventvEmotes.cpp Show resolved Hide resolved
src/providers/seventv/SeventvEmotes.cpp Show resolved Hide resolved
src/providers/seventv/SeventvEmotes.cpp Show resolved Hide resolved
src/providers/seventv/SeventvEmotes.cpp Show resolved Hide resolved
@Nerixyz Nerixyz force-pushed the feat/personal-emotes branch from 35daae1 to d8135fa Compare March 7, 2023 18:57
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/messages/Message.cpp Show resolved Hide resolved
src/messages/Message.cpp Show resolved Hide resolved
src/messages/MessageElement.cpp Show resolved Hide resolved
src/messages/MessageElement.cpp Show resolved Hide resolved
src/messages/MessageElement.cpp Show resolved Hide resolved
src/providers/seventv/SeventvEmotes.cpp Show resolved Hide resolved
src/providers/seventv/eventapi/Dispatch.cpp Show resolved Hide resolved
src/providers/twitch/TwitchChannel.cpp Show resolved Hide resolved
@AnatoleAM
Copy link

Personal emotes are not yet deleted after they haven't been used in a while

This probably should be tied to the socket's lifecycle; i.e clear the data after a successful non-resumed connection. Otherwise, the EventAPI wouldn't send the data again.

@Nerixyz
Copy link
Collaborator Author

Nerixyz commented Mar 8, 2023

This probably should be tied to the socket's lifecycle; i.e clear the data after a successful non-resumed connection. Otherwise, the EventAPI wouldn't send the data again.

I don't think there's much need for this then, compared to the required bookkeeping. The images for the emotes are automatically deleted after a while. And I don't think the emote-sets take that much space (about 1KiB/Set - a 32x32 emote with 25 frames takes about 100KiB). Furthermore, there aren't many reconnects, so this data would even with the bookkeeping stay in memory.

@Nerixyz Nerixyz marked this pull request as ready for review March 8, 2023 11:52
Copy link

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

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

LGTM, i may have some nitpicks later but we can let this cook in nightly for a while and if needed have a subsequent PR

@AnatoleAM AnatoleAM merged commit b84fde0 into SevenTV:chatterino7 Mar 11, 2023
@Nerixyz Nerixyz deleted the feat/personal-emotes branch March 11, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants