chore: remove the quality-tier API nothing drives - #134
Merged
Conversation
Closes #128. Adaptive quality is not planned, so per that issue's decision the dead API goes rather than being wired. setQualityTier() and placementCount had no callers anywhere in src, tests or scripts, and neither was on the SceneRenderer interface, so nothing could reach them polymorphically either. #qualityTier was initialised to "high" and never changed, which meant the tier path in the renderer never executed. Left as-is that is worse than no API: a reader finding setQualityTier() reasonably concludes quality adapts, and placementCount documented two consumers — tests and a diagnostics overlay — that do not exist. The tier machinery underneath is real and stays. Every asset declares a minimumQualityTier, buildSceneLayers filters on it, and tierAllows is tested as a pure function; callers just always pass "high". That is now an explicitly named constant with the reasoning attached, so whoever wires adaptation later finds the filtering already built and only needs something that decides a tier. Also drops "quality tiers" from the delivered-properties list in docs/decisions.md, so the record matches the code. The mention in docs/baseline/README.md stays: that is a field to record when benchmarking, not a claim that adaptation works. The Pixi visual guard confirms the frames are byte-identical, which is the expected result of changing how an unchanged value reaches the layer factory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Reviewed 0b58be3: no findings. The dead setter/getter and mutable field are removed without disturbing marker pooling; the layer factory remains explicitly pinned to the tested high tier; and the delivered-properties record now matches the product decision. Pixi visual guard: 0% changed pixels on all three scenes. Focused asset/layer tests: 38/38 passed. Current CI is green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #128. Adaptive quality is not planned, so per that issue's decision the dead API goes rather than being wired.
What was dead
setQualityTier()andplacementCounthad no callers anywhere insrc,testsorscripts, and neither was on theSceneRendererinterface, so nothing could reach them polymorphically either.#qualityTierwas initialised to"high"and never changed, so the tier path in the renderer never executed.Left in place that is worse than no API. A reader finding
setQualityTier()reasonably concludes quality adapts — it does not.placementCountdocumented two consumers, "tests and the diagnostics overlay", neither of which exists.What stays, and why
The tier machinery underneath is real and works:
lowmediumhighEvery asset declares a
minimumQualityTier,buildSceneLayersfilters on it, andtierAllowsis tested as a pure function. Callers just always pass"high". That is now a named constant with the reasoning attached, so whoever wires adaptation later finds the filtering already built and tested, and only needs something that decides a tier — a frame budget, a device probe, or a user setting.The tiering is also sensibly ordered, which is worth preserving: things a diver navigates by survive, decoration goes first. Dropping the wreck to
lowremoves dressing but leaves every structural landmark.Record corrected
docs/decisions.mdlisted quality tiers among the PixiJS scene's delivered properties. Removed, so the record matches the code.The mention in
docs/baseline/README.mdstays — that is a field to record when benchmarking a device, not a claim that adaptation works.Verification
typecheck,lint,sites:check, 133 unit, 28 parity, 36 e2e.The Pixi visual guard reports 0% changed pixels on all three scenes, which is the expected result of changing how an unchanged value reaches the layer factory — and the first time that guard has been used to confirm a refactor is visually inert rather than to catch a regression.