refactor: decompose start_round() into focused helpers (#390)#437
refactor: decompose start_round() into focused helpers (#390)#437
Conversation
Extract four private methods to reduce start_round() from ~280 to ~65 lines: - _ensure_media_player_service(): lazy MediaPlayerService init - _prepare_intro_round(): all intro flag logic, returns will_defer_for_splash - _build_round_metadata(): metadata dict + background album art task - _initialize_round(): current_song, timers, player resets, phase transition Also update test_lights.py to reflect the subtle intensity behaviour change shipped in #433 (base + offset replaces fixed 0.6× scale). No behaviour change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the start_round method in custom_components/beatify/game/state.py by decomposing it into several private helper methods, including _ensure_media_player_service, _prepare_intro_round, _build_round_metadata, and _initialize_round, to improve code organization. Additionally, it updates a unit test in tests/unit/test_lights.py to align with updated brightness scaling logic. Feedback was provided to simplify the mark_played call in the newly created _initialize_round method by utilizing the validated resolved_uri variable directly.
| will_defer_for_splash: bool, | ||
| ) -> None: | ||
| """Commit all round state: current song, timers, player resets, phase transition.""" | ||
| self._playlist_manager.mark_played(song.get("_resolved_uri") or song.get("uri")) |
There was a problem hiding this comment.
Since resolved_uri is guaranteed to be non-null at this point (due to the check at the beginning of start_round), you can directly use it here. This simplifies the code and avoids the redundant or song.get("uri") which was likely a holdover from before the refactor. This also makes it consistent with how resolved_uri is used to set self.current_song['uri'] on line 1273.
| self._playlist_manager.mark_played(song.get("_resolved_uri") or song.get("uri")) | |
| self._playlist_manager.mark_played(resolved_uri) |
Summary
start_round()was 278 lines handling song selection, media player setup, intro mode detection, playback with retries, metadata init, and round state — all in one method.Extracted four focused helpers:
_ensure_media_player_service(hass)— lazy init_prepare_intro_round(song, hass) → bool— all intro flag logic_build_round_metadata(song, resolved_uri, will_defer) → dict— metadata + async art fetch_initialize_round(song, metadata, resolved_uri, will_defer)— current song, timers, player resets, phasestart_round()is now ~65 lines of orchestration. All 322 tests pass.Also fixes a stale test assertion for subtle intensity (was checking old 0.6× scale, now checks base + offset per #433).
Closes #390
🤖 Generated with Claude Code