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

fix: resolve /live channel liveness using their channel ID #5172

Merged
merged 5 commits into from
Feb 17, 2024

Conversation

KleberPF
Copy link
Contributor

@KleberPF KleberPF commented Feb 12, 2024

This fix for #5158 uses the TwitchChannel roomId() as the message id when sending live messages (TwitchMessageBuilder::liveMessage and TwitchMessageBuilder::liveSystemMessage). I wasn't able to replicate the same logic for live messages sent through NotificationController (I couldn't find a way to access the TwitchChannel from there), so I went with the other option of using a case insensitive string compare. I accept any ideas on how I could do this better, especially the NotificationController bit.

(felanbird): Fixes #5158

@Felanbird
Copy link
Collaborator

Felanbird commented Feb 13, 2024

changelog.md entry
- Bugfix: Fixed an issue where channels who were live before you opened Chatterino, didn't always get greyed-out in the `/live` tab when they went offline. (#5172)

Copy link
Collaborator

@Felanbird Felanbird left a comment

Choose a reason for hiding this comment

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

This seems to work as expected, so far 3 of my channels loaded before displayName capitalization was obtained, have correctly been greyed-out in the /live tab when they went offline

@KleberPF
Copy link
Contributor Author

Thanks for the review. Should I add a commit with the changelog entry?

@Felanbird
Copy link
Collaborator

Thanks for the review. Should I add a commit with the changelog entry?

if you want, otherwise pajlada will do it on the weekend (and maybe move my comma)

@KleberPF
Copy link
Contributor Author

Added a commit because QString::compare returns 0 when the strings are equal

@pajlada pajlada changed the title Use room id as message id when sending live message (#5158) fix: resolve /live channel liveness using their channel ID Feb 17, 2024
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Little bit hard to test but it seems to work as expected - thank you!

@pajlada pajlada enabled auto-merge (squash) February 17, 2024 10:59
@pajlada
Copy link
Member

pajlada commented Feb 17, 2024

Thank you! As a first-time contributor, you can now add yourself to the contributors list that's shown inside the About page in Chatterino.

If you want this, you can open a new PR where you modify the resources/contributors.txt file and add yourself to the list. Make sure to read the comments at the top for instructions.

@pajlada pajlada merged commit 3cdb7bf into Chatterino:master Feb 17, 2024
17 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.

/live channels never resolve when the channel was populated before displayname casing could be obtained
3 participants