[FEAT] 연주 화면 UI 및 기능 구현#48
Conversation
📝 WalkthroughWalkthrough연주 화면을 백킹트랙·메트로놈·피아노 입력·실시간 노트바와 연동하고, 재생 제어 및 녹음 결과 저장 흐름을 추가했다. MIDI 입력 활성화 조건, 피아노 사운드, 레이턴시 측정 화면도 확장했다. Changes연주 화면 기능
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MIDIInput
participant useActiveNotes
participant PracticePlayPage
participant ToneTransport
participant PracticeNoteBars
participant PracticeResultStore
MIDIInput->>useActiveNotes: note-on/note-off 입력
useActiveNotes->>PracticePlayPage: 노트 이벤트 전달
PracticePlayPage->>ToneTransport: 재생 및 연주 시간 기록
PracticePlayPage->>PracticeNoteBars: 실시간 noteBars 렌더링
PracticePlayPage->>PracticeResultStore: 녹음 결과와 latencyMs 저장
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useActiveNotes.ts`:
- Around line 23-30: useActiveNotes의 note-off 처리에서 활성 노트 표시 해제와 연주 이벤트 종료를
분리하세요. enabled=false인 비활성 상태에서는 onNoteOff를 호출하지 말고 Set 정리만 수행하며,
PracticePlayPage의 pause/stop 경로에서 열린 노트를 명시적으로 확정·해제해 일시정지 시간이 노트 길이에 포함되지 않도록
하세요.
In `@src/pages/practice/PracticePlayPage.tsx`:
- Around line 53-61: Update handleNoteOn and the heldRef/noteBars tracking so
repeated note-on events for the same note before note-off preserve every open
bar instead of overwriting the prior id. Adjust the corresponding note-off
lookup to close the appropriate open bar(s), while keeping recordingRef’s
reverse-search behavior and out-of-range note-bar filtering unchanged.
- Around line 99-112: Remove the delayed autoplay path that invokes
startPlayback after a timeout, so it cannot restart playback or reset recording
state later. Trigger startPlayback only from the START control’s user
click/keyboard handler, where Tone.start runs within the user gesture; preserve
the overlay dismissal in that same handler and prevent duplicate starts through
the existing isPlaying flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 34a244c7-700f-4477-a261-3754c0f8b0be
⛔ Files ignored due to path filters (2)
src/assets/change.svgis excluded by!**/*.svgsrc/assets/check.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
src/components/piano/PracticeNoteBars.tsxsrc/hooks/useActiveNotes.tssrc/hooks/useMetronome.tssrc/hooks/useMidi.tssrc/hooks/usePianoSound.tssrc/index.csssrc/pages/latency/LatencyCheckPage.tsxsrc/pages/practice/PracticePlayPage.tsxsrc/pages/practice/PracticeSettingsPage.tsxsrc/pages/practice/components/BackingTrack.tsxsrc/stores/practiceResultStore.tssrc/utils/metronome.ts
| (note) => { | ||
| // note-off는 항상 처리 (눌려있던 음 해제 — stuck 방지) | ||
| setActiveNotes((prev) => { | ||
| const next = new Set(prev); | ||
| next.delete(note); | ||
| return next; | ||
| }), | ||
| }); | ||
| onNoteOff?.(note); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
비활성 상태의 note-off를 연주 결과로 기록하지 마세요.
enabled=false여도 onNoteOff가 실행됩니다. PracticePlayPage.tsx에서는 pause 중 key release가 노트바 종료 시각을 기록하고, 재개 시 시작/종료 시각을 함께 이동시키므로 노트바 길이에 일시정지 시간이 포함됩니다. 활성 노트 표시 해제와 연주 이벤트 종료를 분리하고, pause/stop 시점에 열린 노트를 명시적으로 확정·해제하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useActiveNotes.ts` around lines 23 - 30, useActiveNotes의 note-off
처리에서 활성 노트 표시 해제와 연주 이벤트 종료를 분리하세요. enabled=false인 비활성 상태에서는 onNoteOff를 호출하지 말고
Set 정리만 수행하며, PracticePlayPage의 pause/stop 경로에서 열린 노트를 명시적으로 확정·해제해 일시정지 시간이 노트
길이에 포함되지 않도록 하세요.
| const handleNoteOn = (note: number, velocity = 100) => { | ||
| playNote(note, velocity); // 즉시 소리 (범위와 무관하게 실제 친 음) | ||
| // 녹음: 실제 친 음 전부 기록 (Transport 시각 = 곡 박자 기준) | ||
| recordingRef.current.push({ midi: note, velocity, onSec: Tone.getTransport().seconds, offSec: null }); | ||
| if (noteCenterFraction(note, keyCount) < 0) return; // 노트바는 건반 범위 안만 | ||
| const id = barIdRef.current++; | ||
| heldRef.current.set(note, id); | ||
| setNoteBars((prev) => [...prev, { id, midi: note, startTime: performance.now(), endTime: null }]); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
동일 노트가 note-off 전에 다시 note-on되면 이전 노트바가 고아 상태로 남음
heldRef.current.set(note, id)는 같은 note에 대한 이전 항목을 덮어씁니다. note-off 없이 같은 음이 다시 note-on되면(예: 서스테인 페달, 채터링 등으로 중복 트리거) 이전 바의 id는 heldRef에서 사라지고, 이후 실제 note-off가 와도 최신 id만 종료되어 이전 바는 endTime: null 상태로 영원히 남아 화면에서 계속 자라거나 사라지지 않습니다. recordingRef 쪽은 배열을 역순 탐색해 열린 항목을 정확히 찾으므로 이 문제가 없지만, noteBars/heldRef 쪽만 영향을 받습니다.
🔧 제안 수정안
const handleNoteOn = (note: number, velocity = 100) => {
playNote(note, velocity); // 즉시 소리 (범위와 무관하게 실제 친 음)
// 녹음: 실제 친 음 전부 기록 (Transport 시각 = 곡 박자 기준)
recordingRef.current.push({ midi: note, velocity, onSec: Tone.getTransport().seconds, offSec: null });
if (noteCenterFraction(note, keyCount) < 0) return; // 노트바는 건반 범위 안만
const id = barIdRef.current++;
+ const prevId = heldRef.current.get(note);
+ if (prevId !== undefined) {
+ // 같은 음이 아직 열려 있다면 새로 열기 전에 먼저 닫아 고아 바 방지
+ setNoteBars((prev) => prev.map((b) => (b.id === prevId ? { ...b, endTime: performance.now() } : b)));
+ }
heldRef.current.set(note, id);
setNoteBars((prev) => [...prev, { id, midi: note, startTime: performance.now(), endTime: null }]);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleNoteOn = (note: number, velocity = 100) => { | |
| playNote(note, velocity); // 즉시 소리 (범위와 무관하게 실제 친 음) | |
| // 녹음: 실제 친 음 전부 기록 (Transport 시각 = 곡 박자 기준) | |
| recordingRef.current.push({ midi: note, velocity, onSec: Tone.getTransport().seconds, offSec: null }); | |
| if (noteCenterFraction(note, keyCount) < 0) return; // 노트바는 건반 범위 안만 | |
| const id = barIdRef.current++; | |
| heldRef.current.set(note, id); | |
| setNoteBars((prev) => [...prev, { id, midi: note, startTime: performance.now(), endTime: null }]); | |
| }; | |
| const handleNoteOn = (note: number, velocity = 100) => { | |
| playNote(note, velocity); // 즉시 소리 (범위와 무관하게 실제 친 음) | |
| // 녹음: 실제 친 음 전부 기록 (Transport 시각 = 곡 박자 기준) | |
| recordingRef.current.push({ midi: note, velocity, onSec: Tone.getTransport().seconds, offSec: null }); | |
| if (noteCenterFraction(note, keyCount) < 0) return; // 노트바는 건반 범위 안만 | |
| const id = barIdRef.current++; | |
| const prevId = heldRef.current.get(note); | |
| if (prevId !== undefined) { | |
| // 같은 음이 아직 열려 있다면 새로 열기 전에 먼저 닫아 고아 바 방지 | |
| setNoteBars((prev) => prev.map((b) => (b.id === prevId ? { ...b, endTime: performance.now() } : b))); | |
| } | |
| heldRef.current.set(note, id); | |
| setNoteBars((prev) => [...prev, { id, midi: note, startTime: performance.now(), endTime: null }]); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/practice/PracticePlayPage.tsx` around lines 53 - 61, Update
handleNoteOn and the heldRef/noteBars tracking so repeated note-on events for
the same note before note-off preserve every open bar instead of overwriting the
prior id. Adjust the corresponding note-off lookup to close the appropriate open
bar(s), while keeping recordingRef’s reverse-search behavior and out-of-range
note-bar filtering unchanged.
| const startPlayback = async () => { | ||
| await Tone.start(); // 오디오 잠금 해제 (클릭 핸들러 안에서만 가능) | ||
| totalBeatRef.current = 0; | ||
| recordingRef.current = []; // 처음부터 재생 시 녹음 초기화 | ||
| setIsPlaying(true); | ||
| start(track.bpm, beatsPerBar, (time, bib) => { | ||
| const beat = totalBeatRef.current % totalCells; | ||
| Tone.getDraw().schedule(() => { | ||
| setBeatInBar(bib); | ||
| setCurrentBeat(beat); | ||
| }, time); | ||
| totalBeatRef.current += 1; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching PracticePlayPage.tsx:\n'
fd -a 'PracticePlayPage\.tsx$' . || true
file="$(fd 'PracticePlayPage\.tsx$' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
printf '\nOutline:\n'
ast-grep outline "$file" --view compact || true
printf '\nRelevant lines 70-185:\n'
sed -n '70,185p' "$file" | nl -ba -v70
printf '\nSTART/render snippets:\n'
rg -n "SHOW_START|showStart|Start|startPlayback|useEffect|setTimeout|onClick|handleStart" "$file" -C 3
fi
printf '\nSearch for auto-start patterns nearby:\n'
rg -n "setTimeout\\(|startPlayback\\(|ShowStart|showStart|isPlaying" . -g '*.tsx' -g '*.ts' | head -n 200Repository: Musereview/FE
Length of output: 473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="$1"
printf 'Referenced file exists: %s\n' "$file"
printf '\nRelevant lines 70-185:\n'
sed -n '70,185p' "$file" | awk '{printf "%6d\t%s\n", NR+69, $0}'
printf '\nSTART/render snippets:\n'
rg -n "SHOW_START|showStart|Start|startPlayback|useEffect|setTimeout|onClick|handleStart" "$file" -C 3
printf '\nAuto-start patterns nearby:\n'
rg -n "setTimeout\\(|startPlayback\\(|ShowStart|showStart|isPlaying" . -g '*.tsx' -g '*.ts' | head -n 200 || trueRepository: Musereview/FE
Length of output: 186
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="/home/jailuser/git/src/pages/practice/PracticePlayPage.tsx"
printf 'Referenced file exists: %s\n' "$file"
printf '\nRelevant lines 70-185:\n'
sed -n '70,185p' "$file" | awk '{printf "%6d\t%s\n", NR+69, $0}'
printf '\nSTART/render snippets:\n'
rg -n "SHOW_START|showStart|Start|startPlayback|useEffect|setTimeout|onClick|handleStart" "$file" -C 3
printf '\nAuto-start patterns nearby:\n'
rg -n "setTimeout\\(|startPlayback\\(|ShowStart|showStart|isPlaying" . -g '*.tsx' -g '*.ts' | head -n 200 || trueRepository: Musereview/FE
Length of output: 13279
🌐 Web query:
Tone.js Tone.start user gesture AudioContext auto-play policy documentation
💡 Result:
To comply with modern browser autoplay policies, Tone.js requires the use of Tone.start triggered by a user gesture (e.g., a button click) to initialize the AudioContext [1][2]. Background Browsers block audible audio playback on page load to improve user experience and control [1][3][4]. Under these policies, the Web Audio API's AudioContext begins in a "suspended" state [1][3]. Any attempt to schedule or play audio before the context is in a "running" state will result in silence or scheduling errors [5][2]. How to Implement 1. Wait for a User Gesture: Attach an event listener (like "click" or "keydown") to a UI element, such as a "Play" button [1][6][2]. 2. Call Tone.start: Inside that event handler, invoke Tone.start [1][6]. This function returns a Promise that resolves once the AudioContext has successfully resumed to a "running" state [5][6][2]. Example Code: document.querySelector("button").addEventListener("click", async => { await Tone.start; console.log("Audio is ready"); // You can now safely play sounds or schedule events }); Verification You can check the state of the context at any time using Tone.context.state [1]. It will return "suspended" if the browser is blocking audio, and "running" once the policy requirements have been met [1]. For more detailed control or detection, browsers also provide the Navigator.getAutoplayPolicy API [7].
Citations:
- 1: https://github.com/Tonejs/Tone.js/wiki/Autoplay
- 2: https://www.npmjs.com/package/tone
- 3: https://developer.mozilla.org/en-US/docs/Web/API/Web_Audio_API/Best_practices
- 4: https://developer.chrome.com/blog/autoplay
- 5: https://github.com/Tonejs/tonejs.github.io/blob/master/index.md
- 6: https://tonejs.github.io/docs/15.1.22/functions/start.html
- 7: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/getAutoplayPolicy
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="/home/jailuser/git/src/pages/practice/PracticePlayPage.tsx"
printf 'Relevant startPlayback/handlers and UI:\n'
sed -n '90,148p;180,245p' "$file" | sed -n '1,120p' | awk '{printf "%6d\t%s\n", NR, $0}'Repository: Musereview/FE
Length of output: 5510
自动播放计时器会绕过 Tone.js 用户手势要求并可能重复启动播放
startPlayback() 会在 setTimeout 回调中执行 await Tone.start(),这不符合 Tone.js 应在点击/键盘用户手势中启动音频上下文的指引;同时该计时器忽略 isPlaying,如果用户在 1 秒内点击“播放”或“再开始”,1 秒后还会再次重置 recordingRef / totalBeatRef 并重启 Transport。建议只在 START 区域的用户点击回调中隐藏 overlay 并调用 startPlayback()。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/practice/PracticePlayPage.tsx` around lines 99 - 112, Remove the
delayed autoplay path that invokes startPlayback after a timeout, so it cannot
restart playback or reset recording state later. Trigger startPlayback only from
the START control’s user click/keyboard handler, where Tone.start runs within
the user gesture; preserve the overlay dismissal in that same handler and
prevent duplicate starts through the existing isPlaying flow.
andada-creator
left a comment
There was a problem hiding this comment.
로컬에서 UI랑 메트로놈 동작은 확인했습니다!
건반 입력이나 노트바 생성 부분을 테스트해 보지 못했는데, 혹시 이 부분은 어떻게 테스트를 해보셨는지 궁금합니다
|
|
||
| return ( | ||
| // 컨테이너: width 1510px, padding 12px, justify-between, radius 6px, bg gray-900 | ||
| <div className={`flex w-[1510px] items-center justify-between rounded-[6px] bg-gray-900 p-3 ${className}`}> |

#️⃣ 관련 이슈
📝 작업 요약
연주 시작 화면 전체 구현
🔧 변경 사항
💬 리뷰어 참고 사항
그리고 녹음은 오디오가 아니라 MIDI 이벤트(PlayedNote)예요. 그래서 오디오 녹음은 넘기지 않습니다. 소리는 신디로 생성되니, 같은 녹음을 신디로 재트리거하면 동일하게 재현됩니다. recording을 onSec/offSec 시각에 맞춰 재생하시면 되고, 세기는 velocity/127로 넘기면 돼요. 구현은 usePianoSound 참고하시면 됩니다!
✅ 체크리스트
Summary by CodeRabbit
새 기능
개선