Add fix plan for mid-play pitch change scheduling issues#81
Open
man572142 wants to merge 115 commits into
Open
Add fix plan for mid-play pitch change scheduling issues#81man572142 wants to merge 115 commits into
man572142 wants to merge 115 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`.
…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
…al players When a track is released back to the pool, the source is unrouted from the mixer. Since BroAudio drives loudness through the mixer track (leaving AudioSource.volume at 1.0), an unrouted source would play at full volume directly to the listener. Now the player's real computed level is pushed onto AudioSource.volume on release, and restored to full on reacquire so the mixer resumes control. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BGM transition: gate the fade-out transition on the previous player's active state instead of the transient AudioSource.isPlaying. IsPlaying flickers false while the prior BGM is scheduled/fading or on the swap frame, causing the wrong branch and the old player not stopping. Use IsActive with Unity null semantics. Dominator routing: set TrackType to Dominator before acquiring the mixer track so the dominator pulls from the dominator pool. Previously the track was grabbed while TrackType was still Generic, placing the dominator on a Main-child Track group subject to ducking/effects (and corrupting the pools on recycle). https://claude.ai/code/session_01R8rRKSU64UwCeANPCdAvGM
… BGM transitions Resuming from pause no longer re-acquires a mixer track or rewinds the playhead. The dsp-time-based schedule (end time, loop boundary) is rebased by the pause duration so playback hands over at the correct moment. MusicPlayer.UpdateInstance keeps the current-BGM pointer on the live player across loop/chain iterations, and DoTransition guards against self-stopping or double-recycling when the same player resumes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mechanical extraction of the two yield-free inline blocks in PlayControl into named private methods, keeping control flow and the shared tail identical. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace fixed Rect parameters on PreferencesDrawer.Draw* methods with a shared `(Rect drawPos, Func<Rect, Rect> getRectAndIterateLine)` pair, so both the Preferences window and the Setup Wizard pages can supply their own rect-iteration strategy instead of pre-computing a fixed number of rects per call. Update all call sites accordingly and drop now-unused imports.
Restore the first-iteration warm-up so PlayScheduled is sample-accurate again (fixing the 1st->2nd loop seam), but apply the clip volume up front via SetupClipVolume + UpdateVolume(force) before scheduling so nothing is gated behind the frame wait -- closing the original drift/pop race. Also fix the start-volume decision: HasFadeIn now resolves the clip-level fade against the override/base/clip-setting priority (non-consuming), so it matches whether TryGetFadeIn actually runs. Without this, clips with a clip-level fade-in started at full volume, and Transition.Immediate BGM played silently. Guard the deferred SetupClipVolume on a flag so it runs exactly once even if the prior BGM ends during the warm-up wait.
The Localization clip-assignment path (TrySetClipInTable) only wrote the asset-table entry, skipping the reset that DrawObjectPicker performs on a normal clip swap. The previous clip's Volume/Start/End/Fade carried over, and the cached SerializedTransport kept clamping playback positions to the old clip's length. Mirror the normal path: reset playback settings and invoke OnClipChanged to invalidate the transport cache. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 adds a comprehensive design document (
Docs/FIX_PLAN_MidPlayPitchChange.md) that outlines the root cause, fix strategy, and test plan for a critical bug where mid-playback pitch changes cause scheduled end times to become stale, resulting in audio gaps, cuts, or skipped content at loop seams.Problem
Since sample-accurate loop support was added, playback end times are computed once at start using the initial pitch and never updated when
IAudioPlayer.SetPitch()is called mid-play. This causes:Fix Design (5-step approach)
RecalculateScheduledEndTime()that derives the correct end time from the playhead position (sample-accurate regardless of pitch history) rather than elapsed timeendDspTimevariables with per-frame reads of_playbackEndDspTimeRecalculateScheduledEndTime()after pitch changes (both immediate and fading)Key Implementation Details
newEndDspTime = AudioSettings.dspTime + (remainingSeconds / currentPitch)_nextPlayerseam times are propagated via existingISchedulableAPIsSetScheduledEndTime) are preserved unchangedOut of Scope
https://claude.ai/code/session_01Ppv5dFBJ7EkK3uLWaEViB8