Add real-time speed trend chart to all games via centralized service#63
Add real-time speed trend chart to all games via centralized service#63
Conversation
…History tests Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/60497adb-33a4-4f98-9fc7-52414d0e292d Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
… for consistent display Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/60497adb-33a4-4f98-9fc7-52414d0e292d Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
acrosman
left a comment
There was a problem hiding this comment.
Review all your new tests. Make sure each test tests something (code, functions, scenarios, etc) unique from other tests. Overly large test files are hard to maintain and cause errors. Ensure the coverage meets project requirements without unless extra tests.
| // ── buildPolylinePoints ─────────────────────────────────────────────────────── | ||
|
|
||
| describe('buildPolylinePoints', () => { | ||
| test('returns empty string for an empty array', () => { |
There was a problem hiding this comment.
Three tests for blank inputs is overkill in this context. [] and null seem more than sufficient.
There was a problem hiding this comment.
Removed the undefined test in 7b62a17. The [] and null tests are kept since they exercise different branches of if (!values || values.length === 0).
| // ── getSpeedHistory ─────────────────────────────────────────────────────────── | ||
|
|
||
| describe('getSpeedHistory()', () => { | ||
| it('returns empty array before any rounds', () => { |
There was a problem hiding this comment.
Why are we again testing blank inputs? Is there any valid reason to test this again?
There was a problem hiding this comment.
Removed the "returns empty array before any rounds" test from all 7 game.test.js files in 7b62a17. The beforeEach(() => { initGame(); }) and the "resets to empty after initGame" test already cover that state.
| // ── getSpeedHistory ─────────────────────────────────────────────────────────── | ||
|
|
||
| describe('getSpeedHistory', () => { | ||
| test('returns empty array before any rounds', () => { |
There was a problem hiding this comment.
Why are we again testing blank inputs? Is there any valid reason to test this again? Review all places you run tests for blank and make sure they are useful. Extra tests, that increase the complexity and upkeep on the testing process aren't useful and should be removed.
There was a problem hiding this comment.
Done in 7b62a17 — removed the redundant "empty before any rounds/trials" test from all 7 game test files and the undefined blank-input test from trendChartService.test.js.
Agent-Logs-Url: https://github.com/acrosman/BrainSpeedExercises/sessions/9e7c898a-74ec-4650-aa74-670dd4f30641 Co-authored-by: acrosman <2972053+acrosman@users.noreply.github.com>
Trimmed in 7b62a17: removed 8 tests total — the |
Field of View had a speed threshold trend chart during gameplay; no other game did. This adds equivalent charts to all remaining games using a shared service to avoid duplicating chart geometry and DOM update logic.
Centralized Service
app/components/trendChartService.js(pre-existing, now used by all games) exposes two functions:buildPolylinePoints(values)— scales an arbitrary numeric array into SVG polyline coordinatesrenderTrendChart(els, values, currentValue)— updates polyline, empty-state, and latest-value elementsPer-game changes (object-track, orbit-sprite-memory, otter-stop, sound-sweep)
Each game received identical structural changes:
game.jsspeedHistory[]module-level array, reset ininitGame()speedHistoryafter staircase adjustment in the scoring functiongetSpeedHistory()export returning a defensive copyindex.jsrenderTrendChartfromtrendChartService#<id>-trend-line,#<id>-trend-empty,#<id>-trend-latestupdateTrendChart()called after each round result and inreset()interface.html<section class="game-trend">with SVG polyline chart using shared CSS classesSpeed metrics tracked
speedPxPerSec(px/s)displayDurationMsintervalMssweepDurationMsBug fix
field-of-view/render.jsrenderThresholdTrendwas passingformatMs(currentSoaMs)(a formatted string) as thecurrentValuefallback while populated entries display raw numbers — inconsistent. Changed to pass the raw number; the "ms" label is static HTML.