Convert entity shapes from circles to triangles #16
Conversation
arthurkkp-cog
commented
Sep 3, 2025
arthurkkp-cog
commented
- Replace drawCircle function with drawTriangle for equilateral triangles
- Update drawCellWithName to render triangular player and AI entities
- Convert food items to triangular shapes
- Update minimap rendering to show triangular representations
- Increase triangle sizes by 50% for better visibility
- Maintain proper scaling based on entity scores
- Replace drawCircle function with drawTriangle for equilateral triangles - Update drawCellWithName to render triangular player and AI entities - Convert food items to triangular shapes - Update minimap rendering to show triangular representations - Increase triangle sizes by 50% for better visibility - Maintain proper scaling based on entity scores
|
akkp-windsurf 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-review |
![windsurf-bot[bot]](https://avatars.githubusercontent.com/in/1066231?s=60&v=4){kind=small}
| const size = 6; // Increased size for minimap triangles | ||
| const height = size * Math.sqrt(3) / 2; | ||
|
|
||
| minimapCtx.beginPath(); | ||
| minimapCtx.arc( | ||
| ai.x * scale, | ||
| ai.y * scale, | ||
| 2, | ||
| 0, | ||
| Math.PI * 2 | ||
| ); | ||
| // Top vertex | ||
| minimapCtx.moveTo(ai.x * scale, ai.y * scale - height * 2/3); | ||
| // Bottom left vertex | ||
| minimapCtx.lineTo(ai.x * scale - size/2, ai.y * scale + height * 1/3); | ||
| // Bottom right vertex | ||
| minimapCtx.lineTo(ai.x * scale + size/2, ai.y * scale + height * 1/3); | ||
| minimapCtx.closePath(); | ||
|
|
||
| minimapCtx.fillStyle = COLORS.MINIMAP.OTHER; | ||
| minimapCtx.fill(); | ||
| }); | ||
|
|
||
| // Draw player cells on minimap | ||
| gameState.playerCells.forEach(cell => { | ||
| const size = 9; // Increased size for player cells on minimap | ||
| const height = size * Math.sqrt(3) / 2; | ||
|
|
||
| minimapCtx.beginPath(); | ||
| minimapCtx.arc( | ||
| cell.x * scale, | ||
| cell.y * scale, | ||
| 3, | ||
| 0, | ||
| Math.PI * 2 | ||
| ); | ||
| // Top vertex | ||
| minimapCtx.moveTo(cell.x * scale, cell.y * scale - height * 2/3); | ||
| // Bottom left vertex | ||
| minimapCtx.lineTo(cell.x * scale - size/2, cell.y * scale + height * 1/3); | ||
| // Bottom right vertex | ||
| minimapCtx.lineTo(cell.x * scale + size/2, cell.y * scale + height * 1/3); | ||
| minimapCtx.closePath(); | ||
|
|
||
| minimapCtx.fillStyle = COLORS.MINIMAP.PLAYER; |
There was a problem hiding this comment.
There's duplicate triangle drawing logic in three places: the main drawTriangle function and both minimap rendering sections. Consider creating a reusable helper function for drawing triangles on the minimap to reduce code duplication and make future maintenance easier.
Internal-only debug info
- Category: Optional
- Subjectivity Score: 1
- Thoughts:
This comment points out code duplication in the triangle drawing logic across three places: the maindrawTrianglefunction and both minimap rendering sections. Looking at the diff, I can clearly see this duplication:
- The main
drawTrianglefunction (lines 24-40) - The AI players minimap rendering (lines 137-150)
- The player cells minimap rendering (lines 153-167)
The suggestion to create a reusable helper function is a valid observation based on the visible code. This is a standard software engineering practice to reduce duplication and improve maintainability. The comment is based on objective code patterns visible in the diff, not personal preference.
| minimapCtx.moveTo(ai.x * scale, ai.y * scale - height * 2/3); | ||
| // Bottom left vertex | ||
| minimapCtx.lineTo(ai.x * scale - size/2, ai.y * scale + height * 1/3); | ||
| // Bottom right vertex | ||
| minimapCtx.lineTo(ai.x * scale + size/2, ai.y * scale + height * 1/3); |
There was a problem hiding this comment.
The triangle positioning is inconsistent between the main rendering and minimap. In both cases, the top vertex is positioned at y - height * 2/3, but these coordinate systems have different scales. Consider extracting a common triangle drawing function that accounts for scale to ensure visual consistency.
Internal-only debug info
- Category: Optional
- Subjectivity Score: 2
- Thoughts:
This comment points out that the triangle positioning is inconsistent between the main rendering and minimap. Looking at the diff, I can see that in the maindrawTrianglefunction (line 24-40), the top vertex is positioned aty - height * 2/3. In the minimap rendering (lines 142-146 and 157-161), the same formula is used, but as the comment correctly notes, these coordinate systems have different scales.
The main rendering uses game coordinates transformed to screen coordinates, while the minimap uses a scaled version of game coordinates. This is a valid observation based on the code shown in the diff. The suggestion to extract a common triangle drawing function that accounts for scale is reasonable and would improve code consistency.
This is a technical observation based on the visible code, not a matter of opinion.
| minimapCtx.moveTo(ai.x * scale, ai.y * scale - height * 2/3); | ||
| // Bottom left vertex | ||
| minimapCtx.lineTo(ai.x * scale - size/2, ai.y * scale + height * 1/3); | ||
| // Bottom right vertex | ||
| minimapCtx.lineTo(ai.x * scale + size/2, ai.y * scale + height * 1/3); |
There was a problem hiding this comment.
The triangle orientation in the minimap doesn't match the main view. In both places, the top vertex is positioned at y - height * 2/3, which means the triangles in the minimap are oriented the same way as in the main view. However, since the minimap is a top-down representation, the orientation should be consistent with the main view's coordinate system.
2 participants
