[FEAT] 메트로놈 · 카운트다운 공통 로직 UI 및 기능 구현#28
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTone.js 기반 메트로놈과 Changes메트로놈 및 레이턴시 체크
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PracticeSettingsPage
participant SettingsModal
participant LatencyCheckPage
participant useMetronome
participant ToneTransport
PracticeSettingsPage->>PracticeSettingsPage: 곡의 BPM과 박자 설정
PracticeSettingsPage->>SettingsModal: 레이턴시 체크 콜백 전달
SettingsModal->>SettingsModal: Tone.start() 실행
SettingsModal->>LatencyCheckPage: 레이턴시 체크 화면 이동
LatencyCheckPage->>useMetronome: start(bpm, beatsPerBar, onBeat)
useMetronome->>ToneTransport: 반복 박자 등록
ToneTransport-->>LatencyCheckPage: 박자 및 카운트다운 전달
LatencyCheckPage->>useMetronome: measuring 전환 시 stop()
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🧹 Nitpick comments (1)
src/utils/metronome.ts (1)
11-40: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift전역 Transport 초기화 및 오디오 노드 생성 최적화
현재 코드에는 두 가지 잠재적인 문제가 있습니다:
transport.cancel()은 Transport에 예약된 모든 이벤트를 전역적으로 삭제합니다. 향후 메트로놈과 함께 백킹 트랙 등 다른Tone.Transport이벤트를 함께 재생하게 되면, 메트로놈을 정지할 때 다른 트랙의 이벤트까지 모두 삭제되는 문제가 발생할 수 있습니다.scheduleRepeat이 반환하는 이벤트 ID를 저장해두고transport.clear(eventId)를 사용하는 것이 안전합니다.- 매 박자마다
new Tone.Player를 생성하고 폐기하는 방식은 오디오 스레드에 가비지 컬렉션(GC) 부하를 주어 성능 저하나 오디오 글리치(끊김 현상)를 유발할 수 있습니다. 메트로놈 클릭음은 매우 짧기 때문에Player인스턴스를 한 번만 생성하고 재사용하는 것이 좋습니다.♻️ 제안하는 리팩토링
export function createMetronome() { const transport = Tone.getTransport(); // 음원 버퍼를 미리 로드 (파일은 한 번만 받음) const hiBuffer = new Tone.ToneAudioBuffer('/sounds/click1.mp3'); const loBuffer = new Tone.ToneAudioBuffer('/sounds/click2.mp3'); + // Player 인스턴스를 한 번만 생성하여 재사용 + const hiPlayer = new Tone.Player(hiBuffer).toDestination(); + const loPlayer = new Tone.Player(loBuffer).toDestination(); + + let eventId: number | null = null; return { start(bpm: number, beatsPerBar: number, onBeat: (time: number, beatInBar: number) => void) { - transport.stop(); - transport.cancel(); + if (eventId !== null) { + transport.clear(eventId); + } + // 전역 Transport를 제어해야 하는 경우에만 사용 (다른 로직과 충돌 방지를 위해 확인 필요) + // transport.stop(); let beat = 0; transport.bpm.value = bpm; - transport.scheduleRepeat((time) => { + eventId = transport.scheduleRepeat((time) => { const beatInBar = beat % beatsPerBar; - const buffer = beatInBar === 0 ? hiBuffer : loBuffer; - - if (buffer.loaded) { - // 매 박마다 새 Player 생성 → 겹침 없음, 재생 후 자동 정리 - const player = new Tone.Player(buffer).toDestination(); - player.start(time); - player.onstop = () => player.dispose(); - } + const player = beatInBar === 0 ? hiPlayer : loPlayer; + + if (player.loaded) { + player.start(time); + } onBeat(time, beatInBar); beat += 1; }, '4n'); transport.start(); }, stop() { - transport.stop(); - transport.cancel(); + if (eventId !== null) { + transport.clear(eventId); + eventId = null; + } + transport.stop(); }, dispose() { + if (eventId !== null) { + transport.clear(eventId); + } hiBuffer.dispose(); loBuffer.dispose(); + hiPlayer.dispose(); + loPlayer.dispose(); }, }; }🤖 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/utils/metronome.ts` around lines 11 - 40, Update the metronome start/stop lifecycle to store the event ID returned by scheduleRepeat and clear only that event with transport.clear instead of calling transport.cancel. Create reusable hi- and lo-click Tone.Player instances once, reuse the selected player for each beat, and dispose both players in dispose; remove per-beat Player creation and disposal while preserving onBeat timing and beat selection.
🤖 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/useMetronome.ts`:
- Around line 4-26: Update useMetronome so the Metronome instance is lazily
created when start or stop is invoked, rather than only during render via ref
initialization. Keep the cleanup in the useEffect lifecycle, but ensure it
disposes the current instance and clears the ref so a subsequent Strict Mode
effect setup or callback invocation creates a fresh instance through the same
accessor.
In `@src/pages/latency/LatencyCheckPage.tsx`:
- Around line 58-61: 재시작 버튼에 클릭 핸들러가 연결되지 않았습니다. LatencyCheckPage의 해당 버튼에
handleRestart를 연결하고, 이 함수에서 카운트다운과 메트로놈 상태를 초기화한 뒤 측정 흐름을 intro 단계부터 다시 시작하도록
구현하세요.
---
Nitpick comments:
In `@src/utils/metronome.ts`:
- Around line 11-40: Update the metronome start/stop lifecycle to store the
event ID returned by scheduleRepeat and clear only that event with
transport.clear instead of calling transport.cancel. Create reusable hi- and
lo-click Tone.Player instances once, reuse the selected player for each beat,
and dispose both players in dispose; remove per-beat Player creation and
disposal while preserving onBeat timing and beat selection.
🪄 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: b9bdf97d-35e1-4d55-ad2c-d503c0aaa9b2
⛔ Files ignored due to path filters (5)
public/sounds/click1.mp3is excluded by!**/*.mp3,!public/**public/sounds/click2.mp3is excluded by!**/*.mp3,!public/**src/assets/metronome/metronome-empty.svgis excluded by!**/*.svgsrc/assets/metronome/metronome-full.svgis excluded by!**/*.svgsrc/assets/restart.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
src/components/metronome/MetronomeDots.tsxsrc/components/settings/SettingsModal.tsxsrc/hooks/useMetronome.tssrc/pages/latency/LatencyCheckPage.tsxsrc/pages/practice/PracticeSettingsPage.tsxsrc/stores/settingsStore.tssrc/utils/metronome.ts
| <button className="button-large2 flex h-[60px] w-[175px] items-center justify-center gap-2 rounded-[6px] bg-gray-800 px-3 py-[6px] text-gray-300"> | ||
| 재시작 | ||
| <RefreshIcon className="h-5 w-5" /> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
'재시작' 버튼의 onClick 핸들러가 누락되었습니다.
버튼 UI가 추가되었지만 클릭 시 동작하는 핸들러가 연결되어 있지 않아 아무런 동작을 하지 않습니다.
의도된 것이 아니라면 카운트다운과 메트로놈 상태를 초기화하고 처음부터 다시 시작하는 로직을 연결해야 합니다.
필요하시다면 상태를 초기화하고 측정 흐름을 처음(intro)부터 다시 실행하는 handleRestart 함수 구현을 도와드릴 수 있습니다. 이슈로 등록할까요?
🤖 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/latency/LatencyCheckPage.tsx` around lines 58 - 61, 재시작 버튼에 클릭 핸들러가
연결되지 않았습니다. LatencyCheckPage의 해당 버튼에 handleRestart를 연결하고, 이 함수에서 카운트다운과 메트로놈 상태를
초기화한 뒤 측정 흐름을 intro 단계부터 다시 시작하도록 구현하세요.
There was a problem hiding this comment.
재시작 버튼 동작은 지금은 작동 안하는게 맞습니다~!
| current: number; // 현재 박 (0-based) | ||
| } | ||
|
|
||
| function Metronome({ total, current }: MetronomeProps) { |
There was a problem hiding this comment.
여기 함수명 MetronomeDots로 파일명과 통일 시켜주시면 좋을 것 같아요~!
| useEffect(() => { | ||
| const timer = setTimeout(() => { | ||
| setPhase('countdown'); | ||
| start(bpm, 4, (_time, bib) => { |
There was a problem hiding this comment.
Tone은 오디오를 끊김 없이 내보내려고 실제 소리가 날 시점보다 조금 앞서서 콜백을 돌려서 아래처럼 바꾸는 거 어떨까요?!
import * as Tone from 'tone';
start(bpm, 4, (time, bib) => {
Tone.getDraw().schedule(() => {
setBeatInBar(bib);
setCountdown(4 - bib);
if (bib === 0 && countdownDoneRef.current) {
setPhase('measuring');
stop();
Tone.getDraw().cancel();
}
}, time);
if (bib === 3) countdownDoneRef.current = true; // 상태전이 조건용 ref는 즉시 갱신 OK
});
추가로 Tone.getDraw()는 자체 큐라 stop()으론 안 비워지기 때문에 클린업에서 stop()은 그대로 두고 Tone.getDraw().cancel()을 추가로 호출해 예약된 시각 갱신 콜백을 정리하면 좋을 것 같아요~~!!
There was a problem hiding this comment.
수정했습니다~! 레이턴시 측정 로직 정확도에 오류가 생길 뻔했네요. 중요한 부분 짚어주셔서 감사해요~~!
kxxnayun
left a comment
There was a problem hiding this comment.
수정하신 것까지 확인했습니다! 수고하셨습니당~~!!
#️⃣ 관련 이슈
📝 작업 요약
메트로놈 공통 재생 로직을 구현하고, 이를 활용한 UI(박자 점 표시), 카운트다운, 레이턴시 체크 페이지 컴포넌트 배치까지 마무리
🔧 변경 사항
메트로놈 (공통 로직)
utils/metronome.ts— Tone.getTransport 기반, BPM·박자표(beatsPerBar)별 강박 구분, 클릭음 재생hooks/useMetronome.ts— React 훅 래핑, start/stop을 useCallback으로 고정 (재렌더 시 재시작 방지)진행 점
components/metronome/MetronomeDots.tsx— 박자 진행을 점으로 표시 (현재 박 강조)박자표 연동
레이턴시 체크 화면
💬 리뷰어 참고 사항
✅ 체크리스트
Summary by CodeRabbit