Add hidden Rift Mode for unsolvable puzzle states - #23
Conversation
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 introduces a novel 'Rift Mode' feature to enhance the player experience when a Sudoku puzzle becomes unsolvable due to a player's move. Instead of simply hitting a dead end, players are presented with a unique, interactive event that offers options to recover or restart, adding a layer of depth and intrigue to otherwise frustrating situations. The implementation includes robust logic to accurately detect unsolvable states, a new user interface for interaction, and ensures seamless persistence across game sessions. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Rift Mode' feature to the Shandoku game, adding a modal UI, new game state variables, and logic to detect unsolvable board states and trigger special events. Key changes include new functions for checking board solvability (hasImmediateContradiction, hasAnySolution), managing game state snapshots, handling user interaction locking, and displaying dynamic status messages. The new mode integrates with existing game mechanics like undo/redo, new game creation, and persistence. Review comments highlight opportunities to improve performance by removing redundant hasAnySolution calls in undo and redo functions, and to enhance maintainability by refactoring duplicated contradiction-checking logic and consolidating riftState initialization into a shared constant.
| future.push({grid:cloneGrid(grid),notes:cloneNotes(notes),selected:selected?{...selected}:null,elapsed,notesMode,autoCleanup}); | ||
| restoreSnapshot(history.pop()); | ||
| setStatus('Undid last move.'); | ||
| boardWasSolvable=hasAnySolution(grid); |
There was a problem hiding this comment.
This line is redundant and introduces a significant performance issue. The restoreSnapshot() function called on line 230 already triggers a call to hasAnySolution() via captureLastSolvableSnapshot(), which correctly sets the boardWasSolvable flag. This extra call means the expensive solver function is run twice on every undo operation. The same issue exists in the redo() function. Removing this line will fix the performance issue without affecting logic.
| history.push({grid:cloneGrid(grid),notes:cloneNotes(notes),selected:selected?{...selected}:null,elapsed,notesMode,autoCleanup}); | ||
| restoreSnapshot(future.pop()); | ||
| setStatus('Redid move.'); | ||
| boardWasSolvable=hasAnySolution(grid); |
There was a problem hiding this comment.
This line is redundant and introduces a significant performance issue. The restoreSnapshot() function called on line 238 already triggers a call to hasAnySolution() via captureLastSolvableSnapshot(), which correctly sets the boardWasSolvable flag. This extra call means the expensive solver function is run twice on every redo operation. The same issue exists in the undo() function. Removing this line will fix the performance issue without affecting logic.
| function hasImmediateContradiction(board){ | ||
| for(let r=0;r<GRID_SIZE;r++){ | ||
| const seen=new Set(); | ||
| for(let c=0;c<GRID_SIZE;c++){ | ||
| const v=board[r][c]; | ||
| if(!v) continue; | ||
| if(seen.has(v)) return true; | ||
| seen.add(v); | ||
| } | ||
| } | ||
| for(let c=0;c<GRID_SIZE;c++){ | ||
| const seen=new Set(); | ||
| for(let r=0;r<GRID_SIZE;r++){ | ||
| const v=board[r][c]; | ||
| if(!v) continue; | ||
| if(seen.has(v)) return true; | ||
| seen.add(v); | ||
| } | ||
| } | ||
| for(let br=0;br<GRID_SIZE;br+=3){ | ||
| for(let bc=0;bc<GRID_SIZE;bc+=3){ | ||
| const seen=new Set(); | ||
| for(let r=br;r<br+3;r++){ | ||
| for(let c=bc;c<bc+3;c++){ | ||
| const v=board[r][c]; | ||
| if(!v) continue; | ||
| if(seen.has(v)) return true; | ||
| seen.add(v); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The logic to check for contradictions in rows, columns, and boxes is duplicated three times. To improve maintainability and reduce code repetition, you could extract the duplicate-checking logic into a helper function that operates on an array of values. While this might involve creating temporary arrays for columns and boxes, the improvement in code clarity and ease of future modifications would be substantial, and the performance impact should be negligible for this operation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
🌸 Diff Haiku In puzzle’s deep rift, Generated by spit-the-diff |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
Motivation
Description
hasImmediateContradiction(board)andhasAnySolution(board)and a guarded trigger flowevaluateRiftTrigger()that fires only onorigin==='player-move'.riftState,lastSolvableSnapshot,captureLastSolvableSnapshot(),snapshotCurrentState(),restoreLastSolvableState(), and helpersetInteractionLocked()to guard input during sequences.triggerRiftEvent()(glitch + staged status lines + node discovery), tappable rift-node rendering inrender(), andopenRiftModal()with modal actionsReturn,Restore last solvable, andStart fresh.applyLoadedData(), clear transient Rift UI on lifecycle reset points (newGame(),restoreSnapshot(),applyLoadedData()), add reduced-motion support, and mark rift nodes with a non-color diamond affordance.script.js(core logic, new functions, state, render changes),index.html(rift modal markup),style.css(rift visuals, animation, reduced-motion), andsw.js(cache name bumped tov5so clients pick up static changes).Testing
npm run lint(ESLint overscript.jsandsw.js) and it passed.lastSolvableSnapshotare encoded into the save payload and restored on resume viaapplyLoadedData()(covered by code-path updates and save format change).Codex Task