[BREAKING] Remove HTMLAudioElement fallback from sound system#8636
Merged
Conversation
The engine used to provide a parallel HTMLAudioElement-based implementation of SoundInstance / SoundInstance3d / AudioHandler / Sound for browsers without a Web Audio API (gated via hasAudioContext()). Every browser the engine targets has had Web Audio support for many years, so the fallback was dead code that only covered IE (already unsupported). - Delete src/platform/sound/capabilities.js. - Flatten SoundInstance / SoundInstance3d to the Web Audio path; remove the prototype override blocks and redefined accessors. - Remove IE detection and the new Audio() branch from AudioHandler. - Remove the public Sound.audio property; Sound now accepts only an AudioBuffer. - Simplify SoundComponentSystem.context to return this.manager.context directly. BREAKING: Sound.audio is removed and new Sound(resource) no longer accepts an HTMLAudioElement. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy HTMLAudioElement-based sound fallback path and simplifies the engine’s sound stack to rely solely on Web Audio, aligning implementation with the project’s supported browser set.
Changes:
- Removed
hasAudioContext()capability gating (and deletedsrc/platform/sound/capabilities.js). - Flattened
SoundInstance/SoundInstance3dto Web Audio-only implementations by deleting the fallback prototype overrides and distance falloff JS emulation. - Updated
AudioHandler,Sound, andSoundComponentSystemto remove HTMLAudioElement support and associated branching.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/platform/sound/sound.js | Sound now wraps an AudioBuffer only; removed audio property and simplified duration. |
| src/platform/sound/instance3d.js | Removed non-WebAudio 3D falloff/path and capability checks; simplified imports. |
| src/platform/sound/instance.js | Removed non-WebAudio fallback implementation and capability gating; constructor and lifecycle now assume Web Audio. |
| src/platform/sound/capabilities.js | Deleted Web Audio capability helper (no longer used). |
| src/framework/handlers/audio.js | Removed IE/HTMLAudioElement loading branch; always decodes into AudioBuffer. |
| src/framework/components/sound/system.js | context accessor no longer warns/branches on capability; returns manager.context. |
Comments suppressed due to low confidence (2)
src/platform/sound/sound.js:15
Soundis now constructed with a requiredAudioBuffer, but thebufferfield is still documented asAudioBuffer|undefined. Ifbufferis no longer optional, consider updating the field JSDoc toAudioBufferto keep generated typings and docs consistent with the new API.
/**
* Contains the decoded audio data.
*
* @type {AudioBuffer|undefined}
*/
buffer;
src/platform/sound/instance.js:90
- The
sourcefield is initialized tonull, but the JSDoc annotates it as{AudioBufferSourceNode}(non-null). This likely flows into generated typings; update the type to includenull(or make the field non-nullable by initializing it to an actual node).
/**
* Gets the source that plays the sound resource. Source is only available after calling play.
*
* @type {AudioBufferSourceNode}
*/
source = null;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, constructing a SoundInstance or SoundInstance3d when manager.context was null (i.e. the browser exposes neither AudioContext nor webkitAudioContext) would throw in _initializeNodes() when it called createGain() on null. Guard the construction and public surface so that instances become inert no-ops in that case instead of crashing: - SoundInstance: skip _initializeNodes() in the constructor; play() returns false; pitch setter skips the context-time snapshot; setExternalNodes / clearExternalNodes / _createSource short-circuit. - SoundInstance3d: constructor skips the panner-backed property initialisation after super(); all panner-backed setters / getters guard this.panner and return sensible defaults. AudioHandler already fails cleanly in this case, so the net effect is that the engine degrades gracefully on Web Audio-less environments instead of throwing. Made-with: Cursor
Previously the constructor eagerly probed window for AudioContext / webkitAudioContext, stashed the constructor on a public `AudioContext` field, and emitted a warning. This is redundant work at instantiation time (SoundManager is created on every AppBase, even when no audio is used) and leaks an implementation detail onto the public surface. Move the resolution into the lazy `get context()` accessor so the capability lookup only happens on first access, remove the public `AudioContext` field, and switch the "not supported" message to `Debug.warnOnce` so repeated accesses don't spam the log. Also updates the JSDoc type for `context` from `AudioContext` to `AudioContext|null` to accurately reflect the returned value. Made-with: Cursor
Aligns `SoundManager` with the class-fields convention in AGENTS.md section 15. Pure defaults (`_context = null`, `_userSuspended = false`, `_volume = 1`) are initialised at the declaration; `this`-referencing assignments (`_unlockHandlerFunc`, `listener`) are kept in the constructor, matching the pattern used by `Mouse` / `Keyboard` / `TouchDevice` after the recent hoist refactor. All fields now have a JSDoc `@type` block. Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The engine used to ship a parallel
HTMLAudioElement-based implementation of its sound system for browsers without Web Audio API support, gated byhasAudioContext(). Every browser the engine targets (Chrome 10+, Firefox 25+, Safari 6+, Edge 12+) has had Web Audio for years, so the fallback was dead code that only ever covered IE — which the engine no longer supports.This PR removes the fallback and all the branching / duplicated prototype definitions that existed to support it.
Changes
src/platform/sound/capabilities.js(existed solely to gate the fallback).SoundInstance: flattened the constructor to the Web Audio branch and removed the ~230-lineObject.assign(SoundInstance.prototype, …)override block (duplicateplay/pause/resume/stop/setExternalNodes/clearExternalNodes/getExternalNodes/_createSource/_onLoadedMetadata/_onTimeUpdate/_onManagerDestroy) and the redefinedvolume/pitch/sound/currentTimeaccessors.SoundInstance3d: removed thefallOffJS distance-function fallback and the redefinedposition/maxDistance/refDistance/rollOffFactor/distanceModelaccessors.AudioHandler: removed the IE detection IIFE and thenew Audio()+canplaythroughbranch in_createSound; now uses Web Audio only.Sound: removed the publicaudioproperty; constructor now accepts only anAudioBuffer.SoundComponentSystem.context: no longer emits the "Audio context is not supported" warning — it just returnsthis.manager.context, which already returnsnullif noAudioContext/webkitAudioContextis available.webkitAudioContexthandling inSoundManageris intentionally left alone.Net: +80 / -523 lines across 6 files.
Public API changes (breaking)
Sound.audiois removed.Before
After
SoundComponentSystem#contextno longer logs a warning or returnsnullon the theoretical "no Web Audio" browser. It now returns the underlyingSoundManager.context, which was alreadynullin the edge case where neitherAudioContextnorwebkitAudioContextexist. Behaviour is effectively unchanged on every supported browser.Test plan
npm run lint— cleannpm run build:types— succeeds; verifiedSound.audio,HTMLAudioElement, andhasAudioContextno longer appear inbuild/playcanvas.d.tsnpm test— 1640 passing, 0 failingsound-*examples and confirm 2D / 3D playback still works