Repository navigation
docs: document the per-feature minimum host matrix (splitscreen#69) - #77
Conversation
Splitscreen's features span several core API generations, so a single version string cannot describe them and no minHost is declared. README now carries the per-feature matrix instead: basic split view is the required floor (source-level check at v0.3.0-alpha.1), while LAN sharing (core 03e1c1d, on canonical and fork main), coordinated/offline frame rendering (f7c761c, fork only) and per-player identity (7633211, fork only) each need their own newer host. Because every optional feature is capability-checked, an older core ran the plugin fine but silently: an unsupported LAN relay, a non-exportable panel set and a missing player-context namespace were indistinguishable from a bug. Each now says so once, naming the commit that added the API it wants, and hostFeatures() exposes the same answers to consumers: - _warnOnce() for the three gated features, plus a toast for the relay - _recordHostFrameApi() and _hasPlayerContextApi(), one predicate per feature shared by the probe and the guarded call site, so the probe cannot disagree with the path that uses it - a relay socket that never opens is counted across every attempt, including a WebSocket the browser refuses to construct at all, and diagnosed after three; the reconnect loop is untouched, so a server upgraded mid-share still recovers Bumps to 1.15.0 for the new public API and diagnostics; no minHost is declared (see README 'Why there is no minHost').
Up to standards ✅🟢 Issues
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
All three findings from the review were verified against the code and stood up — fixed in
Task list (1/6 completed)
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge All three findings from the previous review are fixed in Files Reviewed (4 files)
Previous Review Summary (commit 1e41031)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1e41031)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Reviewed by free · Input: 0 · Output: 0 · Cached: 0 |
…itscreen#69) - startLanShare/stopLanShare clear _lanRelayOk, so hostFeatures().lanRelay reports the share in flight instead of a finished one; README and CLAUDE.md state the same contract - _maybeResumeLanShare initialises opened/failures like startLanShare, so the documented _lanShare shape holds on the crash-recovery path too - the beginOfflineRender dedup test now closes each pass before the next one, so it really re-reaches _warnOnce rather than passing on the early return - two tests pin the verdict lifecycle across stop/restart
|
@pullfrog this PR now conflicts with Generated by Claude Code |
|
Merged
|
Closes #69.
Splitscreen's features span several core API generations, so no single version string describes them — and no
minHostis declared. This PR documents the requirement per feature and adds the diagnostics that make an unsupported host distinguishable from a broken one.Documentation (README "Host compatibility")
main?window.createHighway()+ renderer lifecycle, chart/playback accessv0.3.0-alpha.1(source-level check)/ws/sync/{session_id}03e1c1d(core #1030/#1032)renderFrame/renderFrameAt/setExternalFrameDriverf7c761c(fork #81)window.feedBack.playerContexts,player-difficulty.v17633211(fork #83)Verified against both upstreams:
03e1c1dis onmainof got-feedBack/feedBack and the get-flashbacks/feedBack fork;f7c761cand7633211are ancestors of the fork'smainonly. Thev0.3.0-alpha.1floor is a source-level reading (that tag'sstatic/highway.jsexposescreateHighway, thesetRendererlifecycle and everyhw.*method basic mode calls, and itsindex.htmlloads the file as a classic script) — not an end-to-end certification, and the README says so.Also adds "Why there is no
minHost": core's loader exposesminHostas metadata with no frontend enforcement found, so a value would be a claim that doesn't stop an incompatible install — and declaring one before the basic path has been exercised against a released host would advertise something untested (feedBack#102 tracks the core side).Diagnostics
Every optional feature is capability-checked, so an older core ran the plugin fine — but silently. An unsupported relay, a non-exportable panel set and a missing player-context namespace were indistinguishable from a bug:
_warnOnce()names the missing core commit for the coordinated-frame and player-identity paths; the LAN path also toasts, since a share dialog nobody can join needs an on-screen reason.window.slopsmithSplitscreen.hostFeatures()exposes the same answers to consumers — Visual Export already reads it to avoid promising a deterministic frame the host can't produce._hasPlayerContextApi()(a namespace withoutupsertisn't reported as supported) and_recordHostFrameApi()(the export warning gates on exactly whatrenderFrameAt()will then reject).WebSocketthe browser refuses to construct at all (mixed content, blocked scheme) which never reachesonclose; after three it is reported as a missing relay._lanRelayOkstaysnulluntil an attempt exists — a server endpoint can't be feature-detected — and a share that opened once is never misreported for a later drop.Verification
node tests/screen.test.js— 196 tests, 196 pass, 0 fail (5 → 14 new tests in a "Host-feature diagnostics" section: probe baseline, required floor, partial-namespace rejection, all three halves of the frame API, both warning paths, and the LAN never-open / constructor-throw / opened-once routes).node --check screen.jsclean;plugin.jsonparses.Notes
minHostis added — see the README section above.1.14.21→1.15.0for the new public API and the diagnostics. NoCHANGELOG.mdexists in this repo.CLAUDE.mdgained the matching matrix and the rules for working on these paths, so the two docs stay in sync.big-pickle(free) ｜ 𝕏