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

feat(RTLplay): revamp 2.0.0 + features added #8910

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

Conversation

pierrequiroul
Copy link
Contributor

@pierrequiroul pierrequiroul commented Nov 25, 2024

Description

Added features :

  • now displays radio bel rtl (from the same tv group)
  • now correctly displays the new RTL district channel
  • getThumbnail updated to give images a better gradient border
  • new icons (including vinyl for music)
  • changes to the way information is displayed (formatting)
  • slideshow on media page between poster and background image

Changes/fixes/qol:

  • displays new timestamps correctly
  • moved utilities functions to a separate file
  • renamed presence parameters to make them clearer to understand (avoid typos by using them, especially for me lol)
  • reorganized tags to display the 3 most relevant ones first (for the visual of the new extension)
  • added functions getLocalizedAssets (for the ad icon), limitText (largeImageText has a limit of 128 characters)
  • added crop presets for the getThumbnail function (to modify values in a single location)
  • fixed the search page

So, with all that, I modified the version from 1.0.3 to 2.0.0

Previous look:
#8605

Acknowledgements

Screenshots

Proof showing the creation/modification is working as expected

https://imgur.com/a/vFVSzog

Home page:
https://i.imgur.com/djEaucP.png
Search page:
yEIbEEg
1D4vrsb
Media page
2BOqCcI
74PHFUS
qwEgowh
Watching a TV Show (usePresenceName setting)
PMhPaGN
6l7SGZc
Watching a livestream
T8hs6Ma
7dim9PG
Radio livestream (music clip channel)
cC9zaYl
Regular radio player (bel rtl and radio contact websites)
4TKARwR
6jrzBZO
https://i.imgur.com/su6p58R.png

theusaf
theusaf previously approved these changes Nov 26, 2024
@theusaf theusaf requested a review from Bas950 November 26, 2024 22:24
Comment on lines 137 to 142
// Copy of the function in Youtube utils
let cachedTime = 0;
export function adjustTimeError(time: number, acceptableError: number): number {
if (Math.abs(time - cachedTime) > acceptableError) cachedTime = time;
return cachedTime;
}
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @Bas950 !
I wanted to add the features keeping in mind that it works correctly on older versions too. I noticed that with Version: 2.7.0 (ba99913), when a media is played, the extension spammed headless_sessions in the service worker because the start timestamp was updated too fast, resulting in a lot of headless_sessions failing in rate limited. This meant that the change to other pages was much less responsive or was blocking or taking a long time because I guess there's probably a queue set up for status discord changes. So I looked at how other presences handled timestamps, found this function and it solved the problem. The headless_sessions stabilize after a while and only send when necessary, and the page change is more responsive.

I also tested with the latest recoded build Version: 2.7.0 (e9ff721) if this problem existed and it doesn't. So I agree with you and I came to the conclusion that this function was temporary and could be removed in the future. During development I went back and forth between the 2 versions to see if everything worked as I imagined it would in both.

With adjust function
2 7 0_adjust
Without
2 7 0_noadjust

Copy link
Member

Choose a reason for hiding this comment

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

@Timeraa might be able to confirm, but I don't think we will release these presences to the current bundled extension version. So it will be released with the new alpha/beta version.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite sure, I hope to finish 2.7.0 this year but no promises there but the spamming issue still somewhat happens with that function on 2.6.x because of a broken if check

@pierrequiroul
Copy link
Contributor Author

pierrequiroul commented Nov 28, 2024

@Bas950 @theusaf, I removed the adjustTimeError function as requested.
I also added altnames and new tags in the metadata.json since your last reviews because TV Channels and Radio could be interpreted as different services on their own (I doubt but who knows)

@Bas950 Bas950 enabled auto-merge (squash) November 28, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants