Document comb-filtering and scheduled playback incompatibility#77
Open
man572142 wants to merge 101 commits into
Open
Document comb-filtering and scheduled playback incompatibility#77man572142 wants to merge 101 commits into
man572142 wants to merge 101 commits into
Conversation
- Always sequence Start→Loop→End in editor for Chained mode - Replay OFF: single pass through all three stages - Replay ON: loop at Loop stage until toggled off, then play End - Fix CanReplay() bug where _context=None(0) returned true forever - Pass live isReplayEnabled delegate so toggle is detected mid-playback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Change _context from int to PlaybackStage for type-safe comparisons - Replace implicit enum arithmetic (+1) with explicit PlaybackStage.End return - Flatten nested if into if/else if in EntityAudioPreview Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Refactor AudioEntity.HasLoop() to accept explicit defaults so the editor can determine loop/crossfade settings without depending on SoundManager. Both runtime and editor now share this core logic. Add crossfade scheduling to AudioSourcePreviewStrategy: starting from the second clip, each clip begins playing transitionTime before the previous clip ends, with the incoming clip fading in and the outgoing clip fading out over transitionTime, using the seamless fade eases from RuntimeSetting. https://claude.ai/code/session_01Nvvx4jkUL63frJwHXS3mHw fix
When stop is clicked during Chained play mode preview, the end (outro) clip now plays before fully stopping, matching runtime behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…in editor preview
…re-DiRZf' into claude/implement-unity-localization-JgsSz
…eduplicate table lookup Replace 14+ hard-coded Unity Localization property path strings with named constants, cache dropdown GUIContent arrays to eliminate per-frame allocations, and extract shared TryGetAssetTableAndEntry helper to remove duplicate validation between TryGetClipFromTable and TrySetClipInTable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When switching from Localization to Single mode in LibraryManager, the list height changes between Layout and Repaint events. The conditional GUILayout.Space call caused different control counts, triggering: "Getting control 2's position in a group with only 2 controls" Always call GUILayout.Space with Mathf.Max(0f, compensateHeight) to maintain consistent control sequence across IMGUI events. Also add com.unity.localization package (1.5.10) and update build settings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ion mode When the user selects Localization in the MulticlipsPlayMode dropdown, a dialog asks whether to clear all AudioClip references and clip properties. Yes clears every BroAudioClip row via ResetBroAudioClipSerializedProperties; No reverts the dropdown with no changes to the entity. Implemented in ReorderableClips (intercept in OnDrawHeader) and ReorderableClips.Localization (ConfirmSwitchToLocalizationMode), rather than LibraryManagerWindow.LibraryFactory as originally planned, since the mode switch originates in the clips list header. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rty check - Add Debug.LogWarning in LocalizationClipStrategy when no BroAudioClip row matches the active locale (fallback to default playback properties) - Add EditorUtility.SetDirty(table) + AssetDatabase.SaveAssets() after AddAssetToTable in TrySetClipInTable, matching the existing update-entry branch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> extra common code
Adds BroAudio.PreloadLocalizationAssets(SoundID) and BroAudio.ReleaseLocalizationPreload(SoundID) so callers can warm the Addressables cache before playback, eliminating the WaitForCompletion hitch. Locale changes are handled automatically — stale handles are released and re-fetched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PreviewLocalizationClip was using the bare CreatePreviewRequest overload, which carries no transport data. Left-click now builds a SerializedTransport from the per-locale clip SerializedProperty so StartPosition, FadeIn, FadeOut, EndPosition, and per-clip Volume are all respected — matching the behaviour of the normal PreviewAudio path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Guard SyncClipsWithLocales against empty-but-non-null locale list (which occurs during localization system reinit after domain reload), preventing arraySize=0 from destroying all clip data. Also preserve all per-clip properties (FadeIn, FadeOut, StartPosition, EndPosition, Delay, Weight) during locale sync, not just Volume. Remove debug logs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Conflicts resolved in EntityReplayRequest.cs and AudioEntityEditor.cs by combining Chained play mode seamless loop support (isReplayEnabled/_crossfadeTime) with localization clip picker override (clipPickerOverride) — both features are preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix compilation error
Replaced the inline `#if PACKAGE_LOCALIZATION` block in OnDrawHeader with a single `CheckLocalizationMode()` call. The confirmation dialog, clip-clear logic, and the no-clip bypass are now fully encapsulated in ReorderableClips.Localization.cs via `CheckLocalizationMode` and a refactored `ConfirmSwitchToLocalizationMode`.
Replace runtime LocalizationSettings.AvailableLocales with editor-only LocalizationEditorSettings API that loads locales synchronously from Addressable settings, avoiding the empty non-serialized list that occurs outside play mode
… master volume and pitch
…k is available after scheduled wait Unify clip.Delay handling with the scheduling path by converting clip delay into a ScheduledStartTime before playback, removing the separate PlayDelayed() branch. Also acquire the audio track earlier so it's available before the scheduled wait, and clamp PlayScheduled() to the current dspTime to avoid past-time scheduling.
Expose the player's final volume (clipVolume × trackVolume × audioTypeVolume) via a new internal `IAudioPlayer.GetVolume()` method, implemented in `AudioPlayer.Volume.cs`. Wire up null-safe forwarding in `AudioPlayerInstanceWrapper` and a zero-returning stub in `EmptyInstance`. Add a public extension `GetVolume(this IAudioPlayer)` in `BroAudioChainingMethod`.
Replace _localizationTable/_localizationEntry on AudioEntity with a single LocalizedAudioClip field. SoundManager.Localization now keeps one runtime entry per SoundID with a Tracker AssetChanged subscription, an optional PreloadHandle, and per-user wrappers — unifying IsLoaded, Release, Subscribe, and the SelectClip cache path. Hard break on DEV_Unity6: existing entities must re-pick their table/entry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on in-flight handles When locale changes, the previous preload handle becomes invalid. The code was only clearing CurrentClip, leaving IsPreloaded=true, so LoadLocalizedAssetAsync would return the stale invalid handle instead of re-acquiring for the new locale. Add defensive cleanup: if preloaded, call Addressables.Release on valid handles (no-op if already invalidated, but covers in-flight loads), then reset IsPreloaded and PreloadHandle. This mirrors the pre-baef169 pattern and ensures the next load fetches the correct locale's asset. Also prevents wrong-table ReleaseAsset from being called after locale changes, since IsPreloaded is now reset. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…couple strategy Drop LocalizedRuntimeEntry.IsPreloaded and derive loaded state from PreloadHandle.IsValid() directly, eliminating the parallel flag that was the root cause of the stale-handle bug patched in 3ea216f. Fix ghost-entry leak: OnSelectedLocaleChanged now calls MaybeTearDownEntry for preload-only entries (no user handlers) so their Tracker is unsubscribed after a locale switch instead of lingering until app teardown. Decouple LocalizationClipStrategy from SoundManager by injecting a Func<AudioClip> cache-lookup delegate at Inject time. The strategy is now manager-agnostic and unit-testable without a live SoundManager. SoundID drops out of the strategy's field set; AudioEntity wires the lambda. Replace PlayOnAudioLoaded (method under PACKAGE_ADDRESSABLES) with a cached Action<SoundID> PlayOnLocalizedAudioChanged under PACKAGE_LOCALIZATION — correct guard, signals event-handler intent, and avoids per-subscription delegate allocation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace thrown exceptions with Debug.LogError + null return across all IClipSelectionStrategy implementations. Add ClipSelectionContext.EntityName so strategies can include the audio entity name in error messages. Wire EntityName in AudioEntity.SelectClip. Extract ClipListIsNullOrEmpty helper to Utility to avoid repetition.
# Conflicts: # Assets/BroAudio/Runtime/Player/AudioPlayer.Recycling.cs
- Clips tab "Addressables" toggle (UseAddressables): explains it switches clip references between direct and Addressables (AssetReference). - Overall tab "Addressable" toggle: explains it registers the entity asset itself as an Addressable entry, loadable/unloadable at runtime.
Self-contained plan for a future session: re-establish ground truth, run a safety/benefit analysis, and remove Toggle B (entity-asset Addressables registration) if no internal consumers exist, with verification and keep-and-relabel fallbacks.
…tance rule (#73) PlaybackPreference.Position never returned the follow target's position because the base constructor sets _position to a non-null sentinel (GloballyPlayedPosition), making _position.HasValue always true. As a result, Transform-based plays reported their position as global (negativeInfinity), which IsPlayedGlobally treats as a 2D/global play. This caused the DefaultPlaybackGroup [Ignore If Distance Is Greater Than] rule to be bypassed: once a follow-target play became the previous player in the comb-filtering preventer, every subsequent play hit the 'only one is global' escape hatch and skipped the distance check entirely, regardless of the threshold value. Clear _position in the follow-target constructor so Position resolves from the live target each frame. https://claude.ai/code/session_011TYzQaB1omCEtnEz8rUSuC Co-authored-by: Claude <noreply@anthropic.com>
Compile and manual Editor verification (Steps 2-3) not run: this project has no CLI compile/test pipeline; requires the Unity Editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…and clarify tooltip Manual Editor verification (Step 3) not run: requires the Unity Editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nces Manual Editor verification (Step 5) not run: requires the Unity Editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ggle preference Manual Editor verification (Step 4) not run: requires the Unity Editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each tooltip is now one or two short lines. The full explanation lives in the documentation. API hints added: Addressable Audio Clips points to BroAudio.LoadAssetAsync(SoundID); Addressable Entity Asset points to Addressables.LoadAssetAsync<AudioEntity>(address). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> update
…tips-3igJP Add tooltips to Addressables toggles in AudioEntity inspector
Comb-filtering prevention compares game-clock timestamps captured at Play() time, while scheduled playback operates on AudioSettings.dspTime. The two clocks are never reconciled, so scheduled/delayed plays of the same clip are falsely rejected (and can falsely reject immediate plays). Document the findings and an improvement plan to compare audible onsets on the DSP timeline.
PlaybackStartingTime is stamped only after WaitForScheduledStartTime() completes, so it stays 0 while a scheduled play is pending rather than recording the Play() call time. Reframe the findings around the two pre-onset window problems (the 0-sentinel poisoning immediate plays, and synchronous validation ignoring the incoming play's future onset) instead of implying a wrong recorded timestamp.
man572142
force-pushed
the
DEV_Unity6
branch
4 times, most recently
from
c644eda to
bdf1d13
Compare
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
2 participants
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
This PR adds comprehensive documentation of the interaction between the comb-filtering prevention system and scheduled playback, identifying that the two features operate on incompatible timelines and produce false rejections.
Changes
Docs/comb-filtering-scheduled-playback-plan.mdPlaybackStartingTimeremains0until after the scheduled wait completes, causing pending scheduled plays to poison immediate plays and vice-versaContext
This is a findings and planning document that precedes implementation. It serves as a reference for understanding the bug and guides the refactoring needed to make comb-filtering work correctly with scheduled and delayed playback.
https://claude.ai/code/session_015puumshhsEJjFS3Abu8fCk