default user volume is 50 now

This commit is contained in:
2026-02-25 19:32:44 -05:00
parent 012131283c
commit d5010e6fa8
6 changed files with 6 additions and 848 deletions
@@ -1,274 +0,0 @@
# Plan: Replace `GetCurrentTrackInfo` `map[string]interface{}` with a Typed Struct
**Refactoring catalog item:** #9
**Priority:** P2
**Risk:** Low — the player is not in `FEBindings`, so no Wails binding regeneration is needed. All data flows through the event system.
---
## Problem Statement
`player.getCurrentTrackInfoLocked()` returns `map[string]interface{}` — a stringly-typed map with 10 keys. Then `emitTrackChanged()` mutates this map by bolting on 3 additional keys (`trackLength`, `seekPosition`, `trackChangeId`) before emitting it via `runtime.EventsEmit`. This pattern has several issues:
1. **No compile-time safety** — a typo like `"fileName"` vs `"filename"` is a silent bug.
2. **Split construction** — the 13-field payload is built in two places (`getCurrentTrackInfoLocked` builds 10 fields, `emitTrackChanged` appends 3 more via map mutation). The shape of the data is not visible in any single location.
3. **Inconsistent nil-file fallback** — when `p.currentFile == nil`, the returned map has 7 keys (missing `coverArtSmall`, `coverArtMedium`, `coverArtLarge`). The error fallback in `emitTrackChanged` has only 3 keys. Both cases produce maps with incomplete field sets that differ from each other and from the happy path (13 keys).
4. **Missed opportunity for Wails type generation** — if the player were ever added to `FEBindings`, a struct return type would auto-generate TypeScript bindings. Currently the frontend manually maintains a `TrackInfo` interface that must be kept in sync by hand.
5. **Contrast with rest of codebase** — the queue package already uses proper structs with JSON tags (`queue.Track`, `queue.State`, etc.) for all event payloads. The player is an outlier.
---
## Design Decisions & Reasoning
### Decision 1: Define a single `TrackInfo` struct (not two separate types)
The catalog suggests defining a `TrackInfo` struct. A question arises: should `getCurrentTrackInfoLocked` return a "partial" struct (10 fields) while `emitTrackChanged` extends it with 3 more? No — the whole point is to eliminate the mutation pattern. A single struct with all 13 fields is cleaner. The struct represents "everything the frontend needs to know about the current track for the TrackChanged event."
**Reasoning:** A single struct means one source of truth for the shape of the data. The zero values for `TrackLength`, `SeekPosition`, and `TrackChangeID` are naturally `0` in Go, which is semantically correct for "no track loaded" or "error" fallback cases.
### Decision 2: Use `json` struct tags with camelCase keys
The existing map uses camelCase keys (`"fileName"`, `"coverArtSmall"`, etc.). Wails serializes event payloads as JSON. The struct must use `json:"fileName"` tags to preserve the exact same wire format — otherwise the frontend would break.
**Reasoning:** This is a behavioral requirement, not a style choice. The frontend `TrackInfo` interface expects camelCase keys. Changing them would require coordinated frontend changes for zero benefit.
### Decision 3: Keep `getCurrentTrackInfoLocked` but change its return type
Rather than inlining all logic into `emitTrackChanged`, keep the `getCurrentTrackInfoLocked` helper but have it return `TrackInfo` (with the base 10 fields populated). Then `emitTrackChanged` fills in the remaining 3 fields (`TrackLength`, `SeekPosition`, `TrackChangeID`) on the struct before emitting.
**Reasoning:** This preserves the separation of concerns — "build metadata from file/DB" vs "compute playback position and emit." It also keeps `GetCurrentTrackInfo()` (the public method) useful: it returns the same struct, just without the playback-timing fields (which are zero-valued). If the player is ever added to `FEBindings`, this method's return type would auto-generate a TypeScript class.
### Decision 4: Eliminate `GetCurrentTrackInfo()` public method — or keep it?
`GetCurrentTrackInfo()` has **zero Go callers** and **zero TypeScript callers** (the player is not in `FEBindings`). It exists only as dead code. However, it was likely intended as a Wails binding that hasn't been wired up yet, and it could be useful in the future.
**Decision: Keep it.** The cost of a single unused method is minimal, and it now returns a proper struct which would be useful if the player is added to `FEBindings` later. If desired, it can be removed as part of a separate cleanup (item #13 addresses dead player methods).
### Decision 5: Fix the inconsistent nil-file/error fallbacks
Currently:
- **nil file fallback** (line 840-848): returns 7 keys — missing `coverArtSmall`, `coverArtMedium`, `coverArtLarge`
- **error fallback** in `emitTrackChanged` (line 319-323): returns only 3 keys — missing most fields
With a struct, both fallbacks naturally return a fully-populated struct (all fields present, most set to zero values). The `State` field should still be set explicitly in both cases. This eliminates the inconsistency for free.
### Decision 6: Place the struct in the existing `player.go` file, not a new file
The player package has only 3 files (`player.go`, `volume.go`, `player_test.go`). The struct is tightly coupled to the player — it describes what the player emits. Creating a separate `trackinfo.go` file for a single ~20-line struct definition would be premature file splitting for such a small package.
**Reasoning:** Follow the existing pattern — `State` type and playback constants are already defined in `player.go`. The `TrackInfo` struct logically belongs alongside them.
### Decision 7: Use `State` type (not `string`) in the struct
Currently the map stores `string(p.state)` — explicitly converting the `State` type to `string`. The struct should use the `State` type with `json:"state"` tag. Since `State` is `type State string`, JSON serialization produces the same string value. This gives us type safety in Go without changing the wire format.
**Reasoning:** The whole point of this refactoring is compile-time safety. Using `string` in the struct for the state field would undermine that goal.
### Decision 8: Use `uint64` for `TrackChangeID` (match the field type)
The `Player` struct defines `trackChangeID uint64`. The struct field should be `TrackChangeID uint64`. The frontend `TrackInfo` interface uses `number` which can safely represent integers up to 2^53 — more than sufficient for a monotonic counter that starts at 0 per session.
---
## Implementation Plan
### Step 1: Define the `TrackInfo` struct in `player.go`
Add the struct definition near the existing `State` type (around line 58-65), after the sentinel errors:
```go
// TrackInfo contains metadata and playback state for the currently loaded track.
type TrackInfo struct {
FileName string `json:"fileName"`
FilePath string `json:"filePath"`
State State `json:"state"`
Title string `json:"title"`
Artist string `json:"artist"`
Album string `json:"album"`
CoverArt string `json:"coverArt"`
CoverArtSmall string `json:"coverArtSmall"`
CoverArtMedium string `json:"coverArtMedium"`
CoverArtLarge string `json:"coverArtLarge"`
TrackLength int `json:"trackLength"`
SeekPosition int `json:"seekPosition"`
TrackChangeID uint64 `json:"trackChangeId"`
}
```
**Note:** `json:"trackChangeId"` (lowercase `d`) matches the existing frontend interface key `trackChangeId`.
### Step 2: Refactor `getCurrentTrackInfoLocked` to return `TrackInfo`
Change the signature from `(map[string]interface{}, error)` to `TrackInfo` (no error needed — see reasoning below).
**Why remove the error return?** The current function never actually returns an error. It handles all error cases internally (DB lookup failure logs and falls back to defaults). The error in the return signature is unused dead weight. With a struct, the zero-value fallback is even cleaner.
Updated implementation:
```go
func (p *Player) getCurrentTrackInfoLocked() TrackInfo {
info := TrackInfo{
State: p.state,
}
if p.currentFile == nil {
return info
}
info.FileName = filepath.Base(p.currentFile.Name())
info.FilePath = p.currentFile.Name()
info.Title = info.FileName // default title
if p.db != nil {
meta, err := p.db.Queries.GetTrackMetadataByPath(
p.ctx, info.FilePath,
)
if err == nil {
if meta.Title != "" {
info.Title = meta.Title
}
info.Artist = meta.Artist
info.Album = meta.Album
if meta.CoverArtPath != "" {
base := filepath.Base(meta.CoverArtPath)
info.CoverArt = "/covers/" + base
info.CoverArtSmall = "/covers/" +
library.SizedFilename(base, "_sm")
info.CoverArtMedium = "/covers/" +
library.SizedFilename(base, "_md")
info.CoverArtLarge = "/covers/" +
library.SizedFilename(base, "_lg")
}
} else {
p.logger.Debug(
"Could not get track metadata from database",
"path", info.FilePath, "err", err,
)
}
}
return info
}
```
### Step 3: Update `GetCurrentTrackInfo` (public method)
Change return type from `(map[string]interface{}, error)` to `TrackInfo`:
```go
// GetCurrentTrackInfo returns information about the currently loaded track.
func (p *Player) GetCurrentTrackInfo() TrackInfo {
p.mu.Lock()
defer p.mu.Unlock()
return p.getCurrentTrackInfoLocked()
}
```
**Note:** Dropping the error return is safe — there are zero callers of this method.
### Step 4: Refactor `emitTrackChanged` to build the struct directly
Replace map mutation with direct struct field assignment:
```go
func (p *Player) emitTrackChanged() {
if p.ctx == nil {
p.logger.Error("Context is nil, cannot emit event")
return
}
trackInfo := p.getCurrentTrackInfoLocked()
trackLengthSecs, err := p.trackLengthLocked()
if err != nil {
p.logger.Error("Cannot get track length")
}
trackInfo.TrackLength = trackLengthSecs
// Compute current seek position in seconds.
if p.seeker != nil {
speaker.Lock()
trackInfo.SeekPosition = p.seeker.Position() /
int(p.format.SampleRate)
speaker.Unlock()
}
// Increment track change ID so the frontend can detect changes
// even when the same file plays consecutively.
p.trackChangeID++
trackInfo.TrackChangeID = p.trackChangeID
runtime.EventsEmit(p.ctx, events.TrackChanged, trackInfo)
p.logger.Info(
"Emitting TrackChangedEvent with track info",
"trackInfo", trackInfo,
)
}
```
**Key change:** No more error-fallback map with only 3 keys. If `getCurrentTrackInfoLocked()` returns a zero-valued struct (e.g., when no file is loaded), it still has all 13 fields — the frontend receives a complete, predictable shape every time.
### Step 5: Verify `UnloadTrack` emits `nil` (no change needed)
At `player.go:678`, `UnloadTrack` emits:
```go
runtime.EventsEmit(p.ctx, events.TrackChanged, nil)
```
This is correct and intentional — it signals "no track loaded" to the frontend, which handles `null` in `(trackInfo: TrackInfo | null) => { ... }`. No changes needed here.
### Step 6: Run `make lint` and `make test`
Ensure:
- No linting violations (line length, godot, nlreturn, etc.)
- Tests pass (the existing test is integration-only and skips in CI, but the build itself must succeed with `-tags webkit2_41`)
### Step 7: (Optional) Update the frontend `TrackInfo` interface comments
The frontend `TrackInfo` interface in `frontend/src/store/player-store.ts` already matches the struct fields exactly. No field changes are needed. However, a comment noting that it mirrors `player.TrackInfo` from the backend could be helpful for future maintainers:
```typescript
// TrackInfo mirrors the player.TrackInfo struct in the Go backend.
// Fields are serialized as camelCase JSON via struct tags.
export interface TrackInfo {
// ... (existing fields, unchanged)
}
```
---
## Files Changed
| File | Change |
|------|--------|
| `backend/player/player.go` | Add `TrackInfo` struct; refactor `getCurrentTrackInfoLocked`, `GetCurrentTrackInfo`, and `emitTrackChanged` |
| `frontend/src/store/player-store.ts` | Add comment noting Go struct mirror (optional) |
**No other files need changes.** The frontend receives the data via events and the JSON wire format is identical (same keys, same types). No Wails binding regeneration is needed since the player is not in `FEBindings`.
---
## Risks & Mitigations
| Risk | Likelihood | Mitigation |
|------|------------|------------|
| JSON key mismatch after refactoring | Low | The `json` struct tags are set to exactly match the current map keys. Verify by running the app and checking the frontend receives correct data. |
| `slog` logging of struct differs from map | Very low | `slog` will log the struct fields. The output format changes but the information is equivalent. No functional impact. |
| Future addition of `player` to `FEBindings` | N/A | This refactoring *enables* that future change — Wails will auto-generate a `player.TrackInfo` TypeScript class from the struct. |
---
## Verification
1. `make lint` passes
2. `make build-dev` succeeds
3. Manual test: play a track, verify `now-playing` component shows correct title/artist/cover art
4. Manual test: verify seek bar shows correct track length and seek position
5. Manual test: unload track (stop playback, clear queue), verify frontend clears the now-playing display
6. Manual test: play the same track twice consecutively, verify the seek bar resets (trackChangeId detection)
-283
View File
@@ -1,283 +0,0 @@
# Plan: #12 — Move queue frontend→backend communication to Wails bindings
## Goal
Replace the 15 `Request*` events (frontend→backend) with direct Wails bindings while keeping the 4 backend→frontend push events (`QueueChanged`, `QueueIndexChanged`, `QueueModeChanged`, `QueueTracksModified`) intact. This eliminates ~420 lines of handler boilerplate in Go and aligns the queue's communication pattern with playlists.
## Rationale
**Why bindings for frontend→backend (replacing events):**
- Eliminates 420 lines of hand-written type-assertion boilerplate in `handlers.go`
- Provides compile-time type safety — Wails auto-generates typed TypeScript bindings from Go method signatures, so `float64``int` casting, `[]interface{}``[]string` conversion, and `len(data)` validation all disappear
- Adding a new queue operation becomes a 1-file change (add Go method) vs the current 4-file change (Go event constant, TS event constant, Go handler, TS store method)
- Aligns with the playlist pattern, reducing cognitive overhead
**Why keep events for backend→frontend (not replacing with invalidate-and-refetch):**
- The queue's delta system (`QueueTracksModified` with add/insert/remove/move actions) is genuinely good architecture for a data structure that changes frequently during playback
- Avoids unnecessary round-trips — the backend pushes only what changed
- The playlist's invalidate-and-refetch pattern works for playlists (infrequent mutations) but would be wasteful for a queue (changes on every track advance)
## Prerequisites
The queue must be created in `NewYellowJacketApp()` (before `wails.Run()`) rather than in `OnStartup()`, because Wails v2 consumes the `Bind` slice eagerly at startup via reflection. The struct pointer must be non-nil and fully constructed at `Bind` time.
This is safe because `queue.NewQueue()` only needs `logger` and `db` (both already available in `NewYellowJacketApp`). The player dependency and context are set later via `SetPlayer()` and `SetContext()` during `OnStartup`, which is the existing two-phase initialization pattern used by all other bound services.
## Detailed Steps
### Step 1: Move queue construction to `NewYellowJacketApp` and add to `FEBindings`
**File:** `backend/app.go`
In `NewYellowJacketApp()`, after the playlist service is created (~line 104), add:
```go
yjApp.queue = queue.NewQueue(yjApp.logger, yjApp.database)
```
Add the queue to `FEBindings`:
```go
yjApp.FEBindings = []any{
yjApp.FrontendUtil,
yjApp.appConfig,
yjApp.library,
yjApp.playlist,
yjApp.queue,
}
```
In `OnStartup()`, remove `queue.NewQueue(...)` and keep only the deferred initialization:
```go
yj.queue.SetContext(ctx)
yj.queue.SetPlayer(yj.player)
yj.queue.RestoreState()
```
### Step 2: Remove `registerEventHandlers()` and all handler boilerplate
**File:** `backend/queue/handlers.go`
Remove:
- `registerEventHandlers()` — all 16 `runtime.EventsOn` registrations (lines 41-172)
- All 10 `handle*` functions (lines 201-461): `handleSetQueue`, `handleAddToQueue`, `handlePlayNext`, `handleRemoveFromQueue`, `handleRemoveTracksFromQueue`, `handleAddTracksToQueue`, `handleInsertTracksAtIndex`, `handleMoveQueueTracks`, `handlePlayQueueIndex`, `handlePlayTracksNext`
- The two helper functions `toStringSlice` and `toIntSlice` (lines 175-199)
Keep:
- `OnPlaybackFinished()` (lines 11-38) — this is domain logic, not event boilerplate
**File:** `backend/queue/queue.go`
In `SetContext()`, remove the call to `q.registerEventHandlers()`. The method becomes:
```go
func (q *Queue) SetContext(ctx context.Context) {
q.ctx = ctx
}
```
### Step 3: Remove the 15 `Request*` queue event constants
**File:** `backend/events/events.go`
Remove from the "Queue events" const block (lines 38-52):
- `RequestNext`
- `RequestPrevious`
- `RequestSetQueue`
- `RequestAddToQueue`
- `RequestPlayNext`
- `RequestRemoveFromQueue`
- `RequestToggleShuffle`
- `RequestCycleRepeat`
- `RequestAddTracksToQueue`
- `RequestPlayTracksNext`
- `RequestPlayQueueIndex`
- `RequestRemoveTracksFromQueue`
- `RequestInsertTracksAtIndex`
- `RequestMoveQueueTracks`
- `RequestClearQueue`
Keep `RequestPlay` — it's in the "Playback control events" block and is used by the queue's event handler. Since we're removing `registerEventHandlers`, also remove `RequestPlay` from the queue's event handler. But check if `RequestPlay` is still used by the player package first.
> **Note:** `RequestPlay` is currently handled by the queue (in `handlers.go:48`), not the player. After this refactor, the queue's `Play()` method will be callable directly via bindings, so the `RequestPlay` event handler in the queue is no longer needed. However, `RequestPlay` may still be emitted by the frontend for player-related actions — audit all `RequestPlay` usages before removing the constant.
**File:** `frontend/src/events.ts`
Remove the corresponding 15 `Request*` constants from lines 28-42. Keep the 4 backend→frontend queue events (lines 24-27).
### Step 4: Rewrite the queue store to use Wails bindings
**File:** `frontend/src/store/queue-store.ts`
Replace the 14 action methods that call `EventsEmit(Events.Request*)` with direct calls to the auto-generated Wails bindings.
**Before** (example):
```typescript
import { EventsOn, EventsEmit } from '@runtime/runtime';
import { Events } from '../events';
// ...
next(): void {
EventsEmit(Events.RequestNext);
}
setQueue(filePaths: string[], startIndex: number, shuffleStart = false): void {
EventsEmit(Events.RequestSetQueue, filePaths, startIndex, shuffleStart);
}
```
**After** (example):
```typescript
import { EventsOn } from '@runtime/runtime';
import { Events } from '../events';
import * as QueueService from '@go/queue/Queue';
// ...
next(): void {
QueueService.Next();
}
setQueue(filePaths: string[], startIndex: number, shuffleStart = false): void {
QueueService.SetQueue(filePaths, startIndex, shuffleStart);
}
```
Keep the entire `initializeEventListeners()` method unchanged — the 4 backend→frontend event subscriptions (`QueueChanged`, `QueueIndexChanged`, `QueueModeChanged`, `QueueTracksModified`) and the `applyTracksDelta()` logic remain as-is.
Remove the `EventsEmit` import if no longer needed after removing all `Request*` emissions.
**Complete action method mapping** (queue store method → Wails binding):
| Store method | Current event | Wails binding call |
|---|---|---|
| `next()` | `RequestNext` | `QueueService.Next()` |
| `previous()` | `RequestPrevious` | `QueueService.Previous()` |
| `setQueue(filePaths, startIndex, shuffleStart)` | `RequestSetQueue` | `QueueService.SetQueue(filePaths, startIndex, shuffleStart)` |
| `addToQueue(filePath)` | `RequestAddToQueue` | `QueueService.AddTrack(filePath)` |
| `playNext(filePath)` | `RequestPlayNext` | `QueueService.InsertNext(filePath)` |
| `removeFromQueue(position)` | `RequestRemoveFromQueue` | `QueueService.RemoveTrack(position)` |
| `removeTracksFromQueue(positions)` | `RequestRemoveTracksFromQueue` | `QueueService.RemoveTracks(positions)` |
| `addTracksToQueue(filePaths)` | `RequestAddTracksToQueue` | `QueueService.AddTracks(filePaths)` |
| `playTracksNext(filePaths)` | `RequestPlayTracksNext` | `QueueService.InsertNextTracks(filePaths)` |
| `toggleShuffle()` | `RequestToggleShuffle` | `QueueService.ToggleShuffle()` |
| `cycleRepeat()` | `RequestCycleRepeat` | `QueueService.CycleRepeat()` |
| `playAtIndex(index)` | `RequestPlayQueueIndex` | `QueueService.PlayIndex(index)` |
| `insertTracksAtIndex(filePaths, index)` | `RequestInsertTracksAtIndex` | `QueueService.InsertTracksAt(filePaths, index)` |
| `moveTracksInQueue(fromIndices, toIndex)` | `RequestMoveQueueTracks` | `QueueService.MoveQueueTracks(fromIndices, toIndex)` |
| `clearQueue()` | `RequestClearQueue` | `QueueService.Clear()` |
> **Note:** Some store method names don't match Go method names (e.g., `addToQueue` → `AddTrack`, `playNext` → `InsertNext`). The store method names can remain unchanged for API stability — only the implementation changes.
### Step 5: Handle `Play()` specifically
The queue's `Play()` method is currently triggered by the `RequestPlay` event, which is in the "Playback control events" group and is also emitted by `player-controls.ts`. After this refactor:
- The `RequestPlay` event handler in `handlers.go:48` is removed along with all other handlers
- The frontend should call `QueueService.Play()` directly instead of `EventsEmit(Events.RequestPlay)`
Audit all places that emit `RequestPlay`:
- `frontend/src/components/audio-player/controls/player-controls.ts` — the play button emits `RequestPlay`. This should be changed to call `QueueService.Play()` (or more likely, the queue store should expose a `play()` method that delegates to the binding)
If `RequestPlay` has no other consumers after this change, remove the event constant from both `events.go` and `events.ts`.
### Step 6: Regenerate Wails bindings
Run `wails generate module` (or `make dev` which triggers binding generation) to produce the auto-generated files:
- `frontend/wailsjs/go/queue/Queue.js` — JavaScript bridge calling `window['go']['queue']['Queue'][method](...)`
- `frontend/wailsjs/go/queue/Queue.d.ts` — TypeScript declarations with proper types
- `frontend/wailsjs/go/models.ts` — Updated with `queue.Track`, `queue.State`, `queue.RepeatMode`, etc.
> **Important:** The auto-generated TypeScript types will mirror the Go struct JSON tags, so the frontend types already defined in `queue-store.ts` (`QueueTrack`, `QueueState`, `IndexChanged`, `ModeChanged`, `TracksModified`) will have matching auto-generated equivalents in `models.ts`. We should keep the manually-defined types in the store (they're used by the event listeners which still need them) but could optionally import the model types where convenient.
### Step 7: Handle `SetContext` visibility
When a struct is added to Wails `FEBindings`, **all exported methods** become callable from JavaScript. `SetContext(ctx context.Context)` and `SetPlayer(player TrackLoader)` would be exposed, which is undesirable — they're internal lifecycle methods, not frontend API.
Options:
1. **Unexport them** — rename to `setContext`/`setPlayer`. This requires updating `app.go` to call `q.setContext(ctx)` etc. But unexported methods on structs in other packages aren't accessible, so this won't work without making them package-internal.
2. **Create a thin facade struct** — a `Service` (or `API`) struct that embeds or wraps `*Queue` and only exposes the methods the frontend should call. This is the playlist pattern (`playlist.Service`).
3. **Accept the exposure** — Wails will generate bindings for `SetContext` and `SetPlayer`, but the frontend simply won't call them. They'll be inert in the generated JS. This is what happens with `playlist.Service.SetContext` — it's in the generated `Service.js` but never imported by the frontend.
**Recommendation:** Option 3 — accept it. The playlist already has `SetContext` exposed in its generated bindings (`frontend/wailsjs/go/playlist/Service.js:69`) and it's not a problem. Wails bindings are not a security boundary (the frontend and backend are in the same process). The generated bindings are auto-generated artifacts, not a public API. No one will accidentally call `SetContext` from the frontend.
If `SetPlayer` is a concern because `TrackLoader` is an interface type that Wails can't serialize, Wails may skip it or error during binding generation. If so, either unexport `SetPlayer` only, or have `app.go` set it via an unexported package-level function. This needs testing during step 6.
### Step 8: Update `queue-controller.ts` (no changes needed)
The `QueueController` (`frontend/src/store/controllers/queue-controller.ts`) proxies all actions through `queueStore.*()`. Since we're only changing the store's internal implementation (from `EventsEmit` to binding calls), the controller needs zero changes. All 14 action proxy methods remain identical.
### Step 9: Update components that call `queueStore` directly (no changes needed)
These 6 components import `queueStore` and call its action methods:
- `player-controls.ts``queueStore.next()`, `.previous()`, `.toggleShuffle()`, `.cycleRepeat()`
- `track-list.ts``queueStore.setQueue()`, `.addTracksToQueue()`, `.playTracksNext()`
- `cover-grid.ts``queueStore.setQueue()`, `.addTracksToQueue()`, `.playTracksNext()`
- `genres-view.ts``queueStore.setQueue()`, `.addTracksToQueue()`, `.playTracksNext()`
- `artists-view.ts``queueStore.setQueue()`, `.addTracksToQueue()`, `.playTracksNext()`
- `playlist-view.ts``queueStore.setQueue()`, `.addTracksToQueue()`, `.playTracksNext()`
Since the store's public API (method signatures) is unchanged, none of these components need modifications.
**Exception:** `player-controls.ts` currently emits `Events.RequestPlay` directly for the play/pause button (not through the queue store). This specific call site needs to be updated to either:
- Call `queueStore.play()` (add a `play()` method to the store), or
- Call `QueueService.Play()` directly
### Step 10: Run tests, lint, and build
```bash
make test # Verify Go tests pass (especially queue tests)
make lint # Verify linting passes
cd frontend && pnpm exec tsc --noEmit # Verify TypeScript types
make build-dev # Full build to verify Wails binding generation works
```
## Files Modified
| File | Action | Description |
|---|---|---|
| `backend/app.go` | Edit | Move queue construction; add to `FEBindings` |
| `backend/queue/handlers.go` | Major edit | Remove all `handle*` functions, `registerEventHandlers`, `toStringSlice`, `toIntSlice`. Keep only `OnPlaybackFinished` |
| `backend/queue/queue.go` | Edit | Remove `registerEventHandlers()` call from `SetContext` |
| `backend/events/events.go` | Edit | Remove 15 `Request*` queue constants |
| `frontend/src/events.ts` | Edit | Remove 15 `Request*` queue constants |
| `frontend/src/store/queue-store.ts` | Edit | Replace `EventsEmit` action methods with Wails binding calls |
| `frontend/src/components/audio-player/controls/player-controls.ts` | Edit | Replace `RequestPlay` event emission with binding call |
| `frontend/wailsjs/go/queue/Queue.js` | Auto-generated | New file from `wails generate` |
| `frontend/wailsjs/go/queue/Queue.d.ts` | Auto-generated | New file from `wails generate` |
| `frontend/wailsjs/go/models.ts` | Auto-generated | Updated with queue types |
## Files NOT Modified
| File | Reason |
|---|---|
| `backend/queue/emit.go` | Backend→frontend push events are kept as-is |
| `frontend/src/store/controllers/queue-controller.ts` | Proxies through store; no API change |
| `frontend/src/components/queue-panel/queue-panel.ts` | Uses controller; no API change |
| `frontend/src/components/track-list/track-list.ts` | Calls store methods; no API change |
| `frontend/src/components/cover-grid/cover-grid.ts` | Calls store methods; no API change |
| `frontend/src/components/genres-view/genres-view.ts` | Calls store methods; no API change |
| `frontend/src/components/artists-view/artists-view.ts` | Calls store methods; no API change |
| `frontend/src/components/playlist-view/playlist-view.ts` | Calls store methods; no API change |
## Risk Assessment
**Low risk:**
- The queue's public Go methods are already well-tested and have clear type signatures
- The store's public API doesn't change, so no component-level regressions
- The backend→frontend event system is untouched
- The pattern is proven by the playlist package
**Medium risk:**
- `SetPlayer(TrackLoader)` exposure in Wails bindings — Wails may not handle the interface parameter. If binding generation fails, we'll need to unexport `SetPlayer` and wire it via a package-level function or an exported setter that takes concrete types
- `RequestPlay` event has cross-cutting usage in `player-controls.ts` — needs careful auditing to avoid breaking play/pause
## Net Effect
- **~420 lines removed** from `handlers.go` (boilerplate)
- **~20 lines removed** from `events.go` and `events.ts` (15 event constants each)
- **~30 lines changed** in `queue-store.ts` (swap `EventsEmit` for binding calls)
- **~10 lines changed** in `app.go` (move construction, add to bindings)
- **~3 auto-generated files** created/updated by Wails
- Adding a new queue operation goes from a 4-file change to a 1-2 file change
-286
View File
@@ -1,286 +0,0 @@
# Plan: Move player frontend→backend communication to Wails bindings
## Goal
Replace the 4 remaining `EventsEmit` calls (frontend→backend) in `player-store.ts` with direct Wails bindings, eliminating ~90 lines of handler boilerplate in Go. This completes the pattern established by the queue refactoring (#12) — after this change, **all** frontend→backend communication uses Wails bindings.
## Rationale
Same benefits as the queue refactor:
- Eliminates untyped `data[0].(float64)` casting boilerplate
- Provides compile-time type safety via auto-generated TypeScript declarations
- Adding a new player operation becomes a 1-file change (Go method) instead of 4 files
- Completes the architectural consistency — every frontend→backend call uses bindings, every backend→frontend push uses events
## Key Challenge: `speaker.Init()` in constructor
The player is currently created in `OnStartup` (after `wails.Run()`) because `NewPlayer` calls `speaker.Init()` to initialize audio hardware. Wails bindings must be registered before `wails.Run()`, so we need to split the constructor.
**Solution:** Extract `speaker.Init()` into a separate `InitSpeaker()` method. `NewPlayer` creates the struct with all fields initialized (logger, db, state, default format) but does NOT touch audio hardware. `InitSpeaker()` is called during `OnStartup` when hardware is available.
This is safe because:
- `NewPlayer` already initializes all struct fields before `speaker.Init()` runs
- `speaker.Init()` doesn't depend on any struct state — it only uses the sample rate constant
- The player's methods that touch the speaker (`Play`, `Pause`, `Seek`, `LoadFile`) are only called after `OnStartup` completes, so the speaker will always be initialized before any method is invoked via binding
## Detailed Steps
### Step 1: Split `NewPlayer` — extract `InitSpeaker`
**File:** `backend/player/player.go`
Change `NewPlayer` to accept only `logger` and `db` (remove the `ctx` parameter — context is set later via `SetContext`). Remove `speaker.Init()` from the constructor.
Add a new `InitSpeaker() error` method that does the `speaker.Init()` call.
**Before:**
```go
func NewPlayer(ctx context.Context, logger *slog.Logger, db *database.DB) (*Player, error) {
player := &Player{ctx: ctx, logger: logger, db: db, state: Stopped, ...}
err := speaker.Init(...)
if err != nil { return nil, ... }
return player, nil
}
```
**After:**
```go
func NewPlayer(logger *slog.Logger, db *database.DB) *Player {
return &Player{logger: logger, db: db, state: Stopped, ...}
}
func (p *Player) InitSpeaker() error {
err := speaker.Init(p.format.SampleRate, p.format.SampleRate.N(time.Second/10))
if err != nil { return fmt.Errorf("failed to initialize speaker: %w", err) }
return nil
}
```
Note: `NewPlayer` no longer returns an error (struct creation can't fail) and no longer takes `ctx` (set via `SetContext`).
### Step 2: Remove `registerEventHandlers` from player
**File:** `backend/player/player.go`
Delete the entire `registerEventHandlers()` method (lines 154-272) — all 4 `runtime.EventsOn` registrations and their handler closures.
Update `SetContext` to remove the `registerEventHandlers()` call. Keep only the context assignment and state restoration:
```go
func (p *Player) SetContext(ctx context.Context) {
p.mu.Lock()
p.ctx = ctx
p.mu.Unlock()
p.mu.Lock()
p.restoreStateLocked()
p.mu.Unlock()
}
```
Remove the `"fmt"` import if it becomes unused (it was used by `fmt.Sprintf("%T", data[0])` in the handlers). Check if `fmt` is still used elsewhere in the file — yes, it's used in `loadFileLocked`, `seekLocked`, etc. Keep it.
Remove the `"yellowjacket/backend/events"` import — check first. It's used by:
- `registerEventHandlers` (being removed) — uses `events.RequestPause`, `events.RequestLoadFile`, `events.Seek`, `events.RequestSetVolume`
- `emitPlaybackStateChanged` — uses `events.PlaybackStateChanged`
- `emitPlaybackFinished` — uses `events.PlaybackFinished`
- `emitVolumeChanged` — uses `events.VolumeChanged`
- `emitTrackChanged` — uses `events.TrackChanged`
- `seekLocked` — uses `events.SeekFailed`
- `UnloadTrack` — uses `events.TrackChanged`
So `events` import stays (it's still used by the emit helpers).
The `runtime` import also stays (used by emit helpers and `UnloadTrack`).
### Step 3: Update `SetVolume` to include side effects
**File:** `backend/player/player.go`
The current `SetVolume` only calls `setVolumeLocked()`. The event handler also called `emitVolumeChanged()` and `saveState()`. Update `SetVolume` to match what the event handler did:
**Before:**
```go
func (p *Player) SetVolume(desiredVolume UserVolume) error {
p.mu.Lock()
defer p.mu.Unlock()
p.setVolumeLocked(desiredVolume)
return nil
}
```
**After:**
```go
func (p *Player) SetVolume(desiredVolume UserVolume) {
p.mu.Lock()
defer p.mu.Unlock()
p.setVolumeLocked(desiredVolume)
p.emitVolumeChanged()
p.saveState()
}
```
Note: changed return type from `error` to void — `setVolumeLocked` never fails, and this avoids Wails generating a Promise rejection for a method that can't error. Check if any Go code calls `SetVolume` and checks the error — no callers exist (confirmed above).
### Step 4: Update `app.go` — create player early, add to `FEBindings`
**File:** `backend/app.go`
In `NewYellowJacketApp`, create the player early (after db is available):
```go
yjApp.player = player.NewPlayer(yjApp.logger.WithGroup("player"), yjApp.database)
```
Add to `FEBindings`:
```go
yjApp.FEBindings = []any{
yjApp.FrontendUtil,
yjApp.appConfig,
yjApp.library,
yjApp.playlist,
yjApp.queue,
yjApp.player,
}
```
In `OnStartup`, replace player creation with deferred initialization:
```go
if err := yj.player.InitSpeaker(); err != nil {
startupErr = errors.Join(startupErr, fmt.Errorf("could not initialize speaker: %w", err))
}
yj.player.SetContext(ctx)
```
### Step 5: Update player test
**File:** `backend/player/player_test.go`
Update the test to match the new two-phase constructor:
**Before:**
```go
p, err := NewPlayer(context.Background(), slog.Default(), nil)
if err != nil { t.Fatalf(...) }
p.SetContext(t.Context())
```
**After:**
```go
p := NewPlayer(slog.Default(), nil)
if err := p.InitSpeaker(); err != nil { t.Fatalf(...) }
p.SetContext(t.Context())
```
### Step 6: Remove player `Request*` event constants from Go and TS
**File:** `backend/events/events.go`
Remove from "Playback control events" block:
- `RequestPause`
- `RequestLoadFile`
Remove the entire "Seek events" block — `Seek` was only used as a frontend→backend event. Keep `SeekFailed` by moving it elsewhere (e.g., into a "Playback control events" block or its own group).
Remove from "Volume events" block:
- `RequestSetVolume`
**File:** `frontend/src/events.ts`
Remove:
- `RequestPause`
- `RequestLoadFile`
- `Seek`
- `RequestSetVolume`
Keep:
- `PlaybackStateChanged`, `PlaybackFinished` (backend→frontend push)
- `SeekFailed` (backend→frontend push, even though unused — separate issue #15)
- `TrackChanged` (backend→frontend push)
- `VolumeChanged` (backend→frontend push)
### Step 7: Regenerate Wails bindings
Run `wails generate module` to produce:
- `frontend/wailsjs/go/player/Player.js`
- `frontend/wailsjs/go/player/Player.d.ts`
- Updated `frontend/wailsjs/go/models.ts` with `player.TrackInfo`, `player.UserVolume`, etc.
Expected generated bindings for the methods we need:
- `Pause(): Promise<void>` (from `func (p *Player) Pause() error`)
- `LoadFile(arg1: string): Promise<void>` (from `func (p *Player) LoadFile(filePath string) error`)
- `Seek(arg1: number): Promise<void>` (from `func (p *Player) Seek(targetSeconds int) error`)
- `SetVolume(arg1: number): Promise<void>` (from `func (p *Player) SetVolume(desiredVolume UserVolume)`)
Note: `UserVolume` is `type UserVolume int`, so Wails will serialize it as a plain number. The generated TS type will be `number` (or `player.UserVolume` which maps to `number`).
### Step 8: Rewrite `player-store.ts` actions to use Wails bindings
**File:** `frontend/src/store/player-store.ts`
Replace `EventsEmit` action methods with Wails binding calls:
| Store method | Current | After |
|---|---|---|
| `pause()` | `EventsEmit(Events.RequestPause)` | `Player.Pause()` |
| `loadTrack(filePath)` | `EventsEmit(Events.RequestLoadFile, filePath)` | `Player.LoadFile(filePath)` |
| `seek(seconds)` | `EventsEmit(Events.Seek, seconds)` | `Player.Seek(seconds)` |
| `setVolume(level)` | `EventsEmit(Events.RequestSetVolume, level)` | `Player.SetVolume(level)` |
Remove the `EventsEmit` import (only `EventsOn` will be needed).
Add import: `import * as Player from '@go/player/Player';`
The 4 backend→frontend event subscriptions (`PlaybackStateChanged`, `TrackChanged`, `PlaybackFinished`, `VolumeChanged`) remain unchanged.
### Step 9: Run tests, lint, and TypeScript type check
```bash
make test
make lint
cd frontend && pnpm exec tsc --noEmit
cd frontend && pnpm build
```
## Files Modified
| File | Action | Description |
|---|---|---|
| `backend/player/player.go` | Edit | Split `NewPlayer`, add `InitSpeaker`, remove `registerEventHandlers`, update `SetVolume` |
| `backend/app.go` | Edit | Move player creation early, add to `FEBindings`, call `InitSpeaker` in `OnStartup` |
| `backend/player/player_test.go` | Edit | Update test to use new constructor + `InitSpeaker` |
| `backend/events/events.go` | Edit | Remove `RequestPause`, `RequestLoadFile`, `Seek`, `RequestSetVolume` |
| `frontend/src/events.ts` | Edit | Remove same 4 constants |
| `frontend/src/store/player-store.ts` | Edit | Replace `EventsEmit` with Wails binding calls |
| `frontend/wailsjs/go/player/Player.js` | Auto-generated | New |
| `frontend/wailsjs/go/player/Player.d.ts` | Auto-generated | New |
| `frontend/wailsjs/go/models.ts` | Auto-generated | Updated with player types |
## Files NOT Modified
| File | Reason |
|---|---|
| `frontend/src/store/controllers/player-controller.ts` | Proxies through store; no API change |
| `frontend/src/components/audio-player/` | Uses controller/store; no API change |
| All other component files | No direct player store interaction for these actions |
## Risk Assessment
**Low risk:**
- The player's public methods (`Pause`, `LoadFile`, `Seek`) already have the correct behavior — the event handlers were just thin wrappers
- `SetVolume` is the only method that needs side effects added, and it has zero existing callers
- The test is an integration test that skips by default
**Medium risk:**
- `InitSpeaker()` splitting — if any code path calls a player method that touches the speaker before `InitSpeaker()` runs, it will panic. This is safe because all player method calls happen after `OnStartup` completes, but worth being aware of.
- Wails may expose lifecycle methods (`SetContext`, `SetPlaybackFinishedHandler`, `InitSpeaker`) as callable bindings. Same non-issue as queue — these are harmless in generated JS.
## Net Effect
- **~120 lines removed** from `player.go` (event handlers + boilerplate)
- **~8 lines removed** from event constants (Go + TS)
- **~8 lines changed** in `player-store.ts` (swap `EventsEmit` for binding calls)
- **~10 lines changed** in `app.go` (move construction)
- After this change, **zero** `EventsEmit` calls remain in the frontend for backend requests — all frontend→backend communication uses Wails bindings