Closed r0man1337 closed 2 weeks ago
The latest updates on your projects. Learn more about Vercel for Git βοΈ
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
eternum | β Ready (Inspect) | Visit Preview | π¬ Add feedback | Jun 21, 2024 9:28am |
β±οΈ Estimated effort to review [1-5] | 4 |
π§ͺ Relevant tests | No |
π Security concerns | No |
β‘ Key issues to review |
Performance Concerns: The addition of new textures, materials, and billboards could potentially impact performance, especially on lower-end devices. It's crucial to ensure that these additions do not degrade the user experience by causing frame rate drops or increased load times. |
Code Consistency: The use of hardcoded values (e.g., positions, colors) in several components might lead to inconsistencies and difficulties in maintenance. Consider using a centralized configuration or constants file. | |
Possible Bug: The use of Float32Array for positions and colors in InstancedCastles.tsx assumes that all castles have valid positions and ownership data. This could lead to errors if the data is incomplete or incorrect. |
Category | Suggestion | Score |
Performance |
Combine position and color data into a single buffer to enhance rendering performance___ **To improve performance, consider using a single buffer for both positions and colors inthe instanced meshes. This can reduce the number of buffer bindings during rendering.** [client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx [56-57]](https://github.com/BibliothecaDAO/eternum/pull/974/files#diff-5247f68e32ced43fc3431aef5a0ab60aa86291a0169ba2ee5ba33e6843270a75R56-R57) ```diff -const positionsBuffer = new Float32Array(castles.length * 3); -const colorsBuffer = new Float32Array(castles.length * 3); +const buffer = new Float32Array(castles.length * 6); +let offset = 0; +for (let i = 0; i < castles.length; i++) { + buffer.set([x, 8, -y, myColor.r, neutralColor.g, neutralColor.b], offset); + offset += 6; +} ``` Suggestion importance[1-10]: 8Why: Combining position and color data into a single buffer can significantly improve rendering performance by reducing the number of buffer bindings. This is a valuable optimization. | 8 |
Maintainability |
Use a constant for the texture path to improve maintainability___ **Consider using a constant for the texture path to ensure consistency and maintainability.This will help avoid typos and make it easier to update the path in the future if needed.** [client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx [28-32]](https://github.com/BibliothecaDAO/eternum/pull/974/files#diff-5247f68e32ced43fc3431aef5a0ab60aa86291a0169ba2ee5ba33e6843270a75R28-R32) ```diff -const mapLabel = useTexture("/textures/realm_label.png", (texture) => { +const TEXTURE_PATH = "/textures/realm_label.png"; +const mapLabel = useTexture(TEXTURE_PATH, (texture) => { texture.colorSpace = THREE.SRGBColorSpace; texture.magFilter = THREE.LinearFilter; texture.minFilter = THREE.LinearFilter; }); ``` Suggestion importance[1-10]: 7Why: Using a constant for the texture path can improve maintainability by reducing the risk of typos and making future updates easier. However, this change is not critical for functionality. | 7 |
Refactor color assignment to a function to improve readability and reusability___ **Refactor the color assignment logic to a function to enhance code readability andpotential reusability.** [client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx [68-70]](https://github.com/BibliothecaDAO/eternum/pull/974/files#diff-5247f68e32ced43fc3431aef5a0ab60aa86291a0169ba2ee5ba33e6843270a75R68-R70) ```diff -if (castle.isMine) { - colorsBuffer.set([myColor.r, neutralColor.g, neutralColor.b], idx * 3); -} else { - colorsBuffer.set([neutralColor.r, neutralColor.g, neutralColor.b], idx * 3); +function getColor(castle) { + return castle.isMine ? [myColor.r, neutralColor.g, neutralColor.b] : [neutralColor.r, neutralColor.g, neutralColor.b]; } +colorsBuffer.set(getColor(castle), idx * 3); ``` Suggestion importance[1-10]: 6Why: Refactoring the color assignment logic into a function enhances code readability and reusability. This is a good practice for maintainability, though it does not impact functionality. | 6 | |
Possible issue |
Increase the precision of
___
**To avoid potential issues with floating-point precision in WebGL, consider using a higher | 5 |
User description
PR Type
Enhancement, Bug fix
Description
InstancedCastles.tsx
.HexceptionViewScene.tsx
andWorldMapScene.tsx
.MainScene.tsx
.MainScene.tsx
.Changes walkthrough π
InstancedCastles.tsx
Add textures and color coding for castles
client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx
Points
for better visualization.ShardsMines.tsx
Add billboard with shard label texture
client/src/ui/components/models/buildings/worldmap/ShardsMines.tsx
Structures.tsx
Add billboard with hyperstructure label texture
client/src/ui/components/models/buildings/worldmap/Structures.tsx
Army.tsx
Add billboard with army label texture
client/src/ui/components/worldmap/armies/Army.tsx
HexceptionViewScene.tsx
Increase light intensity settings
client/src/ui/modules/scenes/HexceptionViewScene.tsx - Increased light intensity settings.
MainScene.tsx
Adjust ambient light and raycaster settings
client/src/ui/modules/scenes/MainScene.tsx
WorldMapScene.tsx
Increase light intensity settings
client/src/ui/modules/scenes/WorldMapScene.tsx - Increased light intensity settings.
ArmyHitBox.tsx
Adjust army hitbox position and size
client/src/ui/components/worldmap/armies/ArmyHitBox.tsx - Adjusted army hitbox position and size.