Fix timer reset on resume by restoring normalized elapsed time - #26
Conversation
|
🌸 Diff Haiku Timer lost and reset, Generated by spit-the-diff |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the game timer would reset to zero upon resuming a saved game due to missing or inconsistently stored elapsed time data. The changes introduce a robust mechanism to parse, validate, and restore the timer from various potential storage keys, ensuring a consistent and accurate display and restoration of game progress. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the timer reset issue by introducing a normalizeElapsed function to handle invalid or missing timer values, and by adding backward-compatible fallbacks for legacy data keys. The changes are logical and correctly implemented. I have one suggestion to further improve maintainability by extracting the duplicated fallback logic into a helper function.
| notes=data.notes.map(row=>row.map(arr=>new Set(arr))); | ||
| selected=data.selected; | ||
| elapsed=data.elapsed||0; | ||
| elapsed=normalizeElapsed(data.elapsed ?? data.timeElapsed ?? data.timer ?? data.timeSeconds); |
There was a problem hiding this comment.
This logic for retrieving the elapsed time by checking multiple legacy keys is also used on line 956. To improve maintainability and avoid future inconsistencies where one location is updated but the other is not, consider extracting this logic into a dedicated helper function.
For example, you could create a function:
function getPersistedElapsedTime(data) {
return data.elapsed ?? data.timeElapsed ?? data.timer ?? data.timeSeconds;
}And then use it here and on line 956:
// line 576
elapsed = normalizeElapsed(getPersistedElapsedTime(data));
// line 956
timeStat.textContent = formatTime(getPersistedElapsedTime(saved));This would make the code more DRY (Don't Repeat Yourself) and easier to maintain.
Motivation
0when resuming a saved game (issue Timer starts over when you resume game #21).Description
normalizeElapsedhelper to coerce persisted timer values into a safe non-negative integer.formatTimeto usenormalizeElapsedso displayed times are stable for malformed or legacy values.elapsedinapplyLoadedDatausingelapsedwith backward-compatible fallbackstimeElapsed,timer, andtimeSecondsbefore defaulting to zero.Testing
node --check script.jsand it succeeded.Codex Task