Correct sound duration and playback timing

[mvaligursky]

Keep sound duration reporting and playback position consistent with configured start times and the underlying audio resource.

Changes:

• Clamp SoundInstance.duration and SoundSlot.duration to the playable time after the normalized start time.
• Track SoundInstance.currentTime relative to startTime and wrap assigned values immediately.
• Resume paused or seeked sounds at the correct buffer offset and limit non-looping playback to the remaining configured duration.
• Add regression coverage for shorter, equal, longer, non-zero-start, wrapped-start, seek, and pause/resume cases.

API Changes:

• SoundInstance#duration and SoundSlot#duration now return the playable duration after startTime when a duration is configured.
• SoundInstance#currentTime now consistently reports a position relative to startTime, including immediately after assignment and across pause/resume.

slot.startTime = 2;
slot.duration = 6; // Audio asset duration is 4 seconds

slot.duration; // Before: 4
slot.duration; // After: 2

instance.currentTime = 5; // Effective duration is 3 seconds
instance.currentTime; // Before playback: 5
instance.currentTime; // After: 2

[github-actions]

Build size report

This PR changes the size of the minified bundles.

Bundle	Minified	Gzip	Brotli
playcanvas.min.js	2390.5 KB (+0.2 KB, +0.01%)	614.7 KB (+0.1 KB, +0.01%)	477.1 KB (+0.1 KB, +0.03%)
playcanvas.min.mjs	2387.9 KB (+0.2 KB, +0.01%)	613.4 KB (+0.1 KB, +0.01%)	476.3 KB (−0.0 KB, −0.01%)

[mvaligursky]

🤖 Automated PR review — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed.

---

Ran the new tests and lint locally — both clean, and the change is correctly scoped to the getters (_createInstance passes the raw _duration, so no playback config shifts).

One gap, detailed inline: the clamp ignores startTime, so the "getters agree with playback" invariant still doesn't hold whenever startTime > 0. Not a regression (it predates the modulo→clamp change), but it's the same invariant this PR is establishing and it's a one-token extension of the same expression.

Secondary: neither new test suite exercises startTime. The shorter/equal/longer matrix is otherwise thorough, but with startTime left at its default of 0 the interaction is entirely uncovered — which is why the above slips through green. A case like { startTime: 2, duration: 6 } on the 4s fixture would pin it (expected 2, currently 4).

[mvaligursky]

🤖 Automated re-review (commit b36f6a7) — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed.

---

The startTime gap is resolved. I re-ran the getter-vs-playback comparison at this head across 14 configurations and it now agrees in every one — including the loop path, which the commit doesn't touch directly (loopEnd = min(loopStart + _duration, buffer.duration) turns out to be algebraically identical to loopStart + this.duration, so the loop window matches the getter for free). 18 new tests pass, 116 across the adjacent sound suites, lint clean.

The _currentOffset re-basing is a nice catch beyond what I raised: set pitch does this._currentOffset = this.currentTime, and before this commit those two were in different coordinate spaces (buffer-absolute vs startTime-relative), so that assignment was silently wrong whenever startTime > 0. It's now consistent.

Three points, all minor and mostly about surfacing the new semantics rather than the code.

The PR description is now stale. The API Changes section still lists only the original resource-length clamp, but this commit adds two further observable changes:

• The duration getters now subtract the normalized startTime, so the headline example is incomplete — the same slot.duration = 45 on that 38.22s asset returns something different again once startTime > 0.
• set currentTime now normalizes eagerly (see inline). Previously instance.currentTime = 5 with duration = 3 read back as 5 and only wrapped when playback started; it now reads back 2 immediately. The new test asserts this deliberately, so it's clearly intended — it just deserves a line in the API notes, since it changes a property round-trip.

[mvaligursky]

This is the eager normalization worth calling out in the PR's API Changes section. Before this commit the setter stored the raw value and get currentTime returned it verbatim via _startOffset, so currentTime = 5 on a 3s duration read back as 5 and only wrapped at play(). It now reads back 2.

Matching the documented "it will wrap from the beginning" immediately is the better behavior, and the new test pins it — this is purely about the description not mentioning a changed property round-trip.

One ordering consequence, if you think it's worth caring about: because the wrap is applied at assignment time using the duration as of that moment, the caller's original intent is discarded. Setting currentTime = 5 and then widening duration afterwards leaves the offset at 2, whereas the old lazy wrap would have re-derived 5 % newDuration. Both setters restart playback so there's no torn state — it's just that assignment order now matters where it didn't. Probably fine to accept; a @remarks note would cover it.

[mvaligursky]

"the normalized start time" in the new doc wording is carrying more meaning than a reader will unpack. capTime is a modulo wrap, so two non-obvious rules follow:

• startTime = 6 on a 4s sound normalizes to 2 — playback silently begins 2s in rather than erroring or clamping to the end.
• startTime === sound.duration wraps to 0, i.e. asking to start exactly at the end restarts from the beginning (your { startTime: 4, duration: 6 } test encodes this).

Both match what _playAudioImmediate already did with capTime(this._startTime + currentOffset, this._sound.duration), so aligning the getter to it is right. Just worth stating the wrap outright in the JSDoc ("start times beyond the resource length wrap modulo its duration") so the behavior is discoverable from the API docs rather than only from this expression.

[mvaligursky]

This inlines the same rule SoundInstance#duration expresses as capTime(this._startTime, soundDuration) — presumably because capTime is module-private to instance.js. Two encodings of one rule, and the || 0 is load-bearing in both: it converts the NaN from x % 0 when the asset isn't loaded, and collapses the startTime === duration case to 0.

The two getters are now required to agree (that's the invariant this PR establishes), so the duplication is the kind that drifts quietly — a future tweak to one wouldn't fail any test that compares them, since none does. Exporting capTime from instance.js (or lifting it somewhere shared) and calling it from both would make them structurally impossible to diverge.