Skip to content

avm1: Improve behavior of Sound when loadSound() has been called - #23975

Open
ChrisCPI wants to merge 1 commit into
ruffle-rs:masterfrom
ChrisCPI:avm1-loadsound-fixes
Open

avm1: Improve behavior of Sound when loadSound() has been called#23975
ChrisCPI wants to merge 1 commit into
ruffle-rs:masterfrom
ChrisCPI:avm1-loadsound-fixes

Conversation

@ChrisCPI

@ChrisCPI ChrisCPI commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes #5019
Fixes #22572

This fixes several of the behaviors of the Sound object when playing external sound via loadSound().

Some of the key changes:

  • When loadSound is called on a Sound, it appears that it enters a state of being "separated" from its original owner (even when controlling global sound). This means it can no longer control any prior sounds that were added to it via attachSound (and can no longer attach new sounds in this way), and it inherits its own unique sound transform, not attached to any particular display object. This has now been implemented.
  • Added the ability to load sounds synchronously (where execution is paused until load has completed) when loading local sounds on desktop, matching the Flash Player behavior where methods like start() could become immediately available in this case.
    • Additionally, the playback queue has been removed, as this was an incorrect interpretation of the behavior that was just described.
  • It appears that a single Sound object can only load one sound at a time. Trying to call loadSound() again while another sound is being loaded will cancel the load, and onLoad gets called with false given as the success parameter (See the avm1/sound_load_multiple_remote test).

Testing

Several tests added to cover this.

Checklist

  • I, a human, have self-reviewed this PR and fully understand the changes within.
  • I have made or updated tests where possible.
  • All of my commits are properly scoped, compile successfully, and pass all tests.
  • This PR does not make sense to split up into smaller PRs.
  • An LLM was involved in the authoring of this code. (Used in authoring the added set_sound_transform_with_handle function, and the logic for the load IDs to prevent multiple loads at one time on a single Sound)

@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch 2 times, most recently from 442d3c3 to 94276fb Compare June 13, 2026 20:54
Comment thread core/src/avm1/globals/sound.rs Outdated
Comment thread core/src/avm1/globals/sound.rs Outdated
@Lord-McSweeney

Copy link
Copy Markdown
Member

If both is_stopped and transform are only valid for external sounds, I would suggest adding a new struct called ExternalSound or ExternalSoundInfo that holds both pieces of information, and adding a single Option<ExternalSoundInfo> field to SoundData rather than the two new fields.

@Lord-McSweeney Lord-McSweeney added A-avm1 Area: AVM1 (ActionScript 1 & 2) newsworthy T-fix Type: Bug fix (in something that's supposed to work already) labels Jun 14, 2026
@ChrisCPI

Copy link
Copy Markdown
Collaborator Author

I can look into doing that if no one disagrees with the idea.

@ChrisCPI

Copy link
Copy Markdown
Collaborator Author

If both is_stopped and transform are only valid for external sounds, I would suggest adding a new struct called ExternalSound or ExternalSoundInfo that holds both pieces of information, and adding a single Option<ExternalSoundInfo> field to SoundData rather than the two new fields.

@kjarosh Thoughts on doing this idea? I think it could be helpful in using that to check whether the sound is external, rather than checking the transform.

@kjarosh

kjarosh commented Jun 14, 2026

Copy link
Copy Markdown
Member

It depends on how this plays with the loading state, if it's unrelated to the state then yes.

@ChrisCPI

ChrisCPI commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Putting this in draft for now - Need to do some refactoring, as it looks like the idea of the existence of a playback queue for loaded sounds is incorrect, and so I will likely remove it (effectively undoing most of #21163).

What actually appears to be happening is that in FP, local sounds are loaded synchronously, so that when loadSound() is called, it doesn't continue executing until the sound has been loaded. This is why the start() calls appeared to be queued, when they actually are not.

An easy way to observe this is to see that the onLoad listener can be fired in-between two traces before/after a loadSound() call. e.g.:

var s = new Sound();
s.onLoad = function() { trace("onLoad"); }
trace("before");
s.loadSound("sound.mp3");
trace("after");

In the local projector, this outputs:

before
onLoad
after

But when loading a remote file, it becomes asynchronous, and start() calls have no effect before the sound is fully loaded.

@ChrisCPI
ChrisCPI marked this pull request as draft June 16, 2026 01:37
@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch 8 times, most recently from c153c58 to 6db9cb9 Compare July 15, 2026 00:19
@ChrisCPI
ChrisCPI marked this pull request as ready for review July 15, 2026 00:27
@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch from 6db9cb9 to dbb291c Compare July 15, 2026 00:27
@ChrisCPI ChrisCPI added the waiting-on-review Waiting on review from a Ruffle team member label Jul 16, 2026
@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch 2 times, most recently from 8826c78 to 58589dd Compare July 18, 2026 00:08
@ChrisCPI
ChrisCPI requested a review from kjarosh July 18, 2026 16:29
Comment thread core/src/avm1/globals/sound.rs Outdated
Comment thread core/src/loader.rs
@Lord-McSweeney

Copy link
Copy Markdown
Member

Thank you for working on this!

Comment thread core/src/avm1/globals/sound.rs
@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch from 58589dd to cd0e24d Compare August 1, 2026 05:57
Corrects several behaviors of the Sound object when loading external sound. Some of the important changes:
- On desktop, when loading local sounds, execution is paused until the load is completed, matching the behavior of the FP desktop projector.
- The playback queue has been removed. This was a misinterpretation of how loaded sounds behave on the desktop projector.
- Added a separate `ExternalSound` struct, which will be available (and reset) whenever `loadSound` has been called. Stores several things, such as the individual sound transform for the Sound, whether it is streaming, and whether it will autoplay.
- When calling `start()`, per the docs, the second argument (loops) is now ignored when the sound is streaming.
@ChrisCPI
ChrisCPI force-pushed the avm1-loadsound-fixes branch from cd0e24d to 6daf70d Compare August 1, 2026 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-avm1 Area: AVM1 (ActionScript 1 & 2) newsworthy T-fix Type: Bug fix (in something that's supposed to work already) waiting-on-review Waiting on review from a Ruffle team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sound.setVolume should not affect the whole movie if loadSound has been called bug with sound.position issue on imported .mp3s: Conductor Volume game

4 participants