Skip to content

Lily/Video: Update volume functions to interact with the project volume #2034

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

Merged
merged 9 commits into from
May 21, 2025

Conversation

NexusKitten
Copy link
Contributor

@NexusKitten NexusKitten commented Mar 13, 2025

Resolves #2033

Let the video's volume be affected by the project's volume (which can be manipulated through Scratch Addons and Sound Expanded).

The "volume of video" reporter has been updated to only return the video's personal volume, and does not take into account the project's volume.

2025-03-13.17-42-14.mp4

@github-actions github-actions bot added the pr: change existing extension Pull requests that change an existing extension label Mar 13, 2025
@SharkPool-SP
Copy link
Collaborator

I feel like it would be more efficient to iterate through all created videos rather than all skins

@NexusKitten
Copy link
Contributor Author

I feel like it would be more efficient to iterate through all created videos rather than all skins

I wrote it the way I did because I was copying the above and below events-- I'm not really familiar with this extension, and so I don't want to mess around with it too much. I know you've worked on this one in the past, though, so if you know how to do what you're suggesting, help would be appreciated.

@GarboMuffin
Copy link
Member

GarboMuffin commented Mar 14, 2025

It looks like in this pull request the video element's volume doesn't actually change until end of frame. If the project's scripts get stuck for a while that might mean the volume blocks are now noticeably delayed, until the scripts are unstuck

@SharkPool-SP
Copy link
Collaborator

It looks like in this pull request the video element's volume doesn't actually change until end of frame. If the project's scripts get stuck for a while that might mean the volume blocks are now noticeably delayed, until the scripts are unstuck

wouldnt before execute work

@GarboMuffin
Copy link
Member

No that would another frame of delay

Old version would update element volume in middle of frame, not on either end. Not sure if browsers actually process those changes mid frame or if they end up waiting until the end anyways

@yuri-kiss
Copy link
Member

It looks like in this pull request the video element's volume doesn't actually change until end of frame. If the project's scripts get stuck for a while that might mean the volume blocks are now noticeably delayed, until the scripts are unstuck

You could emit an event from the addon similar to the pause addon, that can be listened to instead, this is how Unsandboxed (the mod) does it and it works perfectly fine

@GarboMuffin
Copy link
Member

The situation I described does not involve the addon at all

@GarboMuffin
Copy link
Member

Although yes without an event the slider will always be a tiny bit delayed

Another option is a making media source node and connecting it to the audio engine. That could help make autoplay more reliable

@yuri-kiss
Copy link
Member

The situation I described does not involve the addon at all

I know. The point of using a event would mean you are not waiting for a frame to finish or start, and only when the volume actually updates.

@NexusKitten
Copy link
Contributor Author

!format

@GarboMuffin
Copy link
Member

yes that solves the project i was talking about once you remove the leftover console.log

@NexusKitten
Copy link
Contributor Author

!format

@NexusKitten
Copy link
Contributor Author

!format

@yuri-kiss
Copy link
Member

this is insanely resource in-effecient but since the idea for volume changes to become an event is not happening we may aswell merge this in its current state.

@yuri-kiss yuri-kiss merged commit a2165e7 into TurboWarp:master May 21, 2025
3 checks passed
@GarboMuffin
Copy link
Member

yes its not ideal but we already have this O(n) loop so its not regressing much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: change existing extension Pull requests that change an existing extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project volume doesn't affect video volume
5 participants