Skip to content

Conversation

@zFishStick
Copy link
Contributor

Related to bug #3260 , i made some huge changes the function ImprovedTube.playerPlaybackSpeed .:

  1. I refactored a bit the function, making it more clean to read separating some statements into new methods.
  2. I moved all the regex pattern into the core.js file, in order to keep all the patterns together in one place and not everywhere in the function
  3. I changed the logic about the video speed as follows: 2.1 If the video is "Normal" (So it's not a music or educational video), then the user is able to change the speed of the video through the slider, BUT, if the user enable the 'force playback speed', then the speed of these types of video can have different speed.
    2.2 I should have cover all the possible scenarios about enabling/disabling the speed button (even change video from music to normal), moving the slider with both enabled/disabled button, you will find out ;))

Hope these changes can be of help <3

…less the user switch on the "force speed on music"
@ImprovedTube
Copy link
Member

hi and thank you so much @zFishStick

Regex: I avoid the extra characters |\b vs ','\\b /typing two programming languages at once .
(besides IDEs might add ' or \ automatically and constructing regex from JS might be necessary / helpful if functionality grows beyond what regex can do)

#3272 is not (yet) addressing #3260

@zFishStick
Copy link
Contributor Author

Hellooo @ImprovedTube , happy to help!
I can change the regex patterns, absolutely, I thought it was the right way, thanks for letting me notice it!

For the issue you mentioned (#3272) I didn't get the mention, cause there are separate operations. Hope you can clarify so i can help you :)

Have a good day!

@zFishStick
Copy link
Contributor Author

Ok done @ImprovedTube ! Probably you were referring to this regex:

			music_tags: new RegExp([
				', (lyrics|remix|song|music|AMV|theme song|full song),',
				'\\(Musical Genre\\)',
				', jazz',
				', reggae'
			].join('|'), 'i'),

I modified it a bit, and now it's like this

			music_tags: new RegExp([
				'\\b(lyrics|remix|song|music|AMV|theme song|full song)\\b',
				'\\(Musical Genre\\)',
				'\\bjazz\\b',
				'\\breggae\\b'
			].join('|'), 'i'),

I removed the ' and replaced with \\b , if you need more help or other changes don't hesitate to ask.
Small note, i made a new feature which was requested by an user, the issue is this one #3252 , if you want to check it it would be great! :)

@ImprovedTube ImprovedTube added Later Temporarily delayed untested please test. (also applies to proactively merged pull requests.) labels Oct 26, 2025
@ImprovedTube
Copy link
Member

hi! :) it is a great exercise that you started all this. Sorry,the original code of this feature is so unnecessarily dense /overwhelming. Besides that, the original Regex Literals are shorter and easier without \, so it its hard to decide what to do.
I didn't notice / review yet, if there is another fix or change of functionality? (#3260 has not been addressed yet it seems)

@zFishStick
Copy link
Contributor Author

I see, yeah i moved all the regex in core.js in order to gather them to have a clear view about the music patterns, but if you want i can do the opposite moving again into the original function.

About issue #3260 , doesn't work yet? When i tested it everything was working fine like i mentioned in my first comment (Point 3 of the list). I can give it another look in case :)

@zFishStick
Copy link
Contributor Author

@ImprovedTube I found the issue! Tonight i will change a bit the function, probably it has been a change of the extension which may have changed the speed behaviour. I'm already working on it, i will update you! :)

@zFishStick
Copy link
Contributor Author

@ImprovedTube I just managed to solve the issue! I made a pull request where i did the following changes:

  • I refactored A LOT the playback function, in order to be more clear and easily readable.
  • I fixed the problem with Music videos and check whether the "Force playback speed" button is active, so now, if i got everything correct, should work.
    I write here some user cases, if you want i can share them :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Later Temporarily delayed untested please test. (also applies to proactively merged pull requests.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants