youtube icon indicating copy to clipboard operation
youtube copied to clipboard

Bug fix related to video speed

Open zFishStick opened this issue 2 months ago • 8 comments

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

zFishStick avatar Oct 18 '25 18:10 zFishStick

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

ImprovedTube avatar Oct 21 '25 01:10 ImprovedTube

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 avatar Oct 21 '25 06:10 zFishStick

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! :)

zFishStick avatar Oct 21 '25 21:10 zFishStick

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)

ImprovedTube avatar Oct 26 '25 20:10 ImprovedTube

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 avatar Oct 26 '25 20:10 zFishStick

@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 avatar Nov 01 '25 09:11 zFishStick

@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 :)

zFishStick avatar Nov 02 '25 13:11 zFishStick

hi! if possible let's separate refactoring and bug fixes to prioritize review (bug-fix/es in what line/s (if any)?) thanks!

ImprovedTube avatar Dec 06 '25 21:12 ImprovedTube