fix: auto-start matching game on Play Again#24
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the "Play Again" button in the matching game required users to manually click Start before the game began. The game now automatically starts after refreshing, providing a smoother replay experience.
Changes:
- Version bumped from 1.0.8 to 1.0.9
- Enhanced the
refreshGame()function to dynamically extract the initialization function name and automatically trigger the start button
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| setup.py | Updated version from 1.0.8 to 1.0.9 |
| games/static/js/src/matching.js | Modified refreshGame() to extract init function name dynamically and auto-click start button after initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setTimeout(function() { | ||
| $('.matching-start-button', element).click(); | ||
| }, 100); |
There was a problem hiding this comment.
The hardcoded 100ms delay for triggering the start button click is a magic number that lacks documentation. This arbitrary timeout could cause race conditions if the initialization takes longer than 100ms on slower devices or network conditions. Consider either documenting why 100ms is sufficient, or implementing a more robust mechanism such as waiting for a specific initialization event or checking for element readiness before triggering the click.
| setTimeout(function() { | |
| $('.matching-start-button', element).click(); | |
| }, 100); | |
| // Automatically start the game once the start button is available. | |
| var $startButton = $('.matching-start-button', element); | |
| if ($startButton.length) { | |
| $startButton.click(); | |
| } else { | |
| // Poll briefly for the button in case initialization is asynchronous. | |
| var attempts = 0; | |
| var maxAttempts = 20; // ~2 seconds with 100ms interval | |
| var pollInterval = 100; | |
| var pollId = setInterval(function() { | |
| var $btn = $('.matching-start-button', element); | |
| if ($btn.length) { | |
| clearInterval(pollId); | |
| $btn.click(); | |
| } else if (++attempts >= maxAttempts) { | |
| clearInterval(pollId); | |
| console.warn('Matching start button not found after waiting for initialization.'); | |
| } | |
| }, pollInterval); | |
| } |
| eval(scriptContent); | ||
| if (typeof MatchingInit === 'function') { | ||
| MatchingInit(runtime, element); | ||
| var match = scriptContent.match(/function\s+(\w+)\s*\(/); |
There was a problem hiding this comment.
The regex pattern used to extract the function name is too simplistic and may fail in edge cases. For example, it won't handle function names with leading/trailing whitespace variations, comments within the function declaration, or multiline function declarations. Consider using a more robust pattern or adding validation to ensure the extracted function name matches expected patterns.
| var match = scriptContent.match(/function\s+(\w+)\s*\(/); | |
| var match = scriptContent.match(/function\s+(?:\/\*[\s\S]*?\*\/\s*|\/\/[^\n]*\n\s*)*([A-Za-z_$][0-9A-Za-z_$]*)\s*\(/m); |
| console.error('Init function not found:', initFuncName); | ||
| window.location.reload(); | ||
| } | ||
| } else { | ||
| console.error('Could not extract function name'); | ||
| window.location.reload(); |
There was a problem hiding this comment.
Falling back to a full page reload on initialization failure may result in poor user experience and potential infinite reload loops if the underlying issue persists. The error handling should provide more user-friendly feedback or retry mechanisms rather than silently reloading the page, which could disorient users and lose their context.
| if (typeof window[initFuncName] === 'function') { | ||
| window[initFuncName](runtime, element); | ||
| setTimeout(function() { | ||
| $('.matching-start-button', element).click(); |
There was a problem hiding this comment.
The selector '.matching-start-button' may not exist immediately after the initialization, or could match multiple elements if the DOM structure contains duplicates. Consider adding validation to ensure the button exists and is unique before attempting to trigger the click event, or handle the case where the button is not found gracefully.
| $('.matching-start-button', element).click(); | |
| const $startButtons = $('.matching-start-button', element); | |
| if ($startButtons.length === 1) { | |
| $startButtons.click(); | |
| } else if ($startButtons.length > 1) { | |
| console.warn('Expected a single .matching-start-button but found', $startButtons.length); | |
| $startButtons.first().click(); | |
| } else { | |
| console.warn('No .matching-start-button found for auto-start; game will not be started automatically.'); | |
| } |
Description
Jira
Bump Version