SendPlayerStateAsync must not overwrite the applied static delay

[chrisuthe]

Found while diagnosing why a server-side sync app concluded an SDK player could not accept a measured delay. Two defects on one parameter.

1. The 0.0 default overwrote a server-set delay

Probed against a real message sequence rather than read off the signature:

2 after server set_static_delay 250:    sync=250  saved=[250]
  client/state ... "static_delay_ms":250 ...
4 after SendPlayerStateAsync(60,false): sync=250  saved=[250]
  client/state ... "static_delay_ms":0   ...     <- overwrote it

The spec is explicit about why this is destructive rather than merely redundant:

The initial message MUST include all state fields. In subsequent messages, the client MAY send only the fields that have changed; the server MUST merge each update into existing state, retaining the last value of any field that is absent.

Omitting an unchanged field is safe — the server retains it. Sending 0 is an overwrite the server is obliged to honour. And the two-argument call is the form MIGRATION-5.4.0.md documents for a volume change, so the first volume nudge after any calibration wiped it.

2. A supplied delay was reported but never applied or persisted

3 after SendPlayerStateAsync(...,400):  sync=250  saved=[250]
  client/state ... "static_delay_ms":400 ...     <- told the server 400, still scheduling 250

The server calibrated against a delay playback was not using, nothing was written to IStaticDelayStore, and the next reconnect reported 250 again — the measurement evaporated with no error anywhere.

The spec both permits and constrains this:

Clients must persist static_delay_ms locally across reboots and server reconnections. Clients may update static_delay_ms ... when audio output changes

A client-initiated update is legitimate; one that does not persist is not an update. So a report-only parameter had no valid meaning.

The change

Task SendPlayerStateAsync(int volume, bool muted, double? staticDelayMs = null);   // was double = 0.0

null leaves the delay untouched. A value applies it to the clock synchronizer, persists it via IStaticDelayStore, and reports it — the same three steps the inbound set_static_delay handler already performed. The reported delay is now always the applied one, never the argument, so the two paths cannot diverge again.

Composes with #169: a negative delay still schedules audio later and still never reaches the wire.

Verification

• Clean dotnet build -c Release --no-incremental: 0 errors, both library TFMs.
• Full suite: 756 passed, 0 failed (751 + 5 — count checked).
• Each half proven to discriminate separately, since one fix could otherwise mask the other:
  • Reporting the caller's parameter again → 2 of 5 fail.
  • Removing the apply-and-persist block → 3 of 5 fail.
• OmittedDelay_ChangesNothingAndPersistsNothing is the positive control in both directions: it catches a supplied delay being ignored and an omitted one wrongly writing through.

Not a bug: the advertisement location

The app's original symptom is app-side. Per spec, player@v1_support.supported_commands is "subset of: 'volume', 'mute'" and the client/state player one is "subset of: 'set_static_delay'" — so set_static_delay can only ever be advertised via client/state, for any conformant client. A server reading only client/hello concludes no player supports it. Documented in the NuGet README alongside the note that a player's initial client/state is deferred until clock sync converges.