Score decay feature #12
Conversation
Implemented score decay mechanics where cells gradually lose score over time. Larger cells decay faster based on their size, adding a strategic element to gameplay. Added comprehensive unit tests for the feature.
|
Christina Pan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
/windsurf |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Other comments (3)
- static/js/renderer.js (6-17) The hexToRgb function doesn't handle 3-digit hex colors (like #FFF). Consider adding support for this format to ensure all valid hex colors work correctly.
- static/js/entities.js (265-265) The `applyScoreDecay` function sets a minimum score of `MIN_SPLIT_SCORE / 2`, but this could create entities that are too small to split but still exist in the game. Consider if this is the intended behavior or if entities should be removed when they get too small.
- static/js/renderer.js (55-56) The alpha calculation logic seems incorrect. With `pulseIntensity` ranging from -0.2 to 0.2, `1 - pulseIntensity` will be between 0.8 and 1.2, making the `Math.max(0.6, ...)` check redundant since the value will never go below 0.6.
1 file skipped due to size limits:
- package-lock.json
| .windsurf/rules/do-not-make-new-test-files.md | ||
| .windsurf/workflows/test-new-changes.md | ||
| .gitignore | ||
| .windsurf/rules/minimal-rules.md |
There was a problem hiding this comment.
You've added .gitignore to the .gitignore file on line 46. This will cause git to ignore the .gitignore file itself, which means future changes to this file won't be tracked. This is generally not recommended as it would make it difficult to maintain consistent ignore rules across the repository.
| const currentTime = Date.now(); | ||
| const deltaTime = (currentTime - lastUpdateTime) / 1000; // Convert to seconds |
There was a problem hiding this comment.
There's an issue with the lastUpdateTime variable. It's reset in updatePlayer() with lastUpdateTime = currentTime, but then updateAI() calculates deltaTime using the same variable. This will result in deltaTime being 0 or very small in updateAI() if it's called right after updatePlayer(), causing minimal score decay for AI players.
| if (score < MIN_SPLIT_SCORE) { | ||
| const pulseIntensity = 0.2 * Math.sin(Date.now() / 200); // Pulsing every 200ms | ||
| const alpha = Math.max(0.6, 1 - pulseIntensity); | ||
| ctx.fillStyle = color.startsWith('rgba') ? color : `rgba(${hexToRgb(color)}, ${alpha})`; |
There was a problem hiding this comment.
There's no validation before converting colors with hexToRgb. If a color is already in rgba format or is invalid, this could cause errors. Consider adding a check to verify the color is a valid hex format before conversion.
|
/windsurf-review |
There was a problem hiding this comment.
Other comments (1)
- static/js/renderer.js (6-17) The hexToRgb function doesn't handle 3-digit hex colors (like #FFF). Consider adding support for these shorthand hex codes to ensure all valid CSS colors work properly.
💡 To request another review, post a new comment with "/windsurf-review".
| } | ||
| } | ||
|
|
||
| let lastUpdateTime = Date.now(); |
There was a problem hiding this comment.
The lastUpdateTime variable is initialized at line 169 but then immediately overwritten in the updatePlayer function. This could cause the first frame's deltaTime calculation to be incorrect (potentially 0). Consider removing the global initialization or using it properly for the first frame.
| .windsurf/rules/do-not-make-new-test-files.md | ||
| .windsurf/workflows/test-new-changes.md | ||
| .gitignore | ||
| .windsurf/rules/minimal-rules.md |
There was a problem hiding this comment.
The .gitignore file is being added to itself (line 46). This is unusual and can cause unexpected behavior with Git. A file shouldn't typically ignore itself as this can lead to confusion when tracking changes.
| const currentTime = Date.now(); | ||
| const deltaTime = (currentTime - lastUpdateTime) / 1000; // Convert to seconds |
There was a problem hiding this comment.
The updateAI function is using lastUpdateTime which is updated in updatePlayer. This creates a dependency where updateAI must be called after updatePlayer to get the correct deltaTime. Consider passing deltaTime as a parameter or calculating it independently in each function.
3 participants
FYI: Testing the Windsurf bot