Conversation
abcxff
requested review from
c86ec23b-fef1-4979-b2fa-b9adc351b8cc and
Copilot
There was a problem hiding this comment.
Pull request overview
This pull request fixes duplicate entity ticks and corrects the hash grid implementation. The changes address critical bugs in the collision detection system and entity management that could cause incorrect game behavior.
Changes:
- Fixed hash grid collision pair tracking by implementing triangular matrix indexing, reducing memory usage from O(n²) to O(n²/2)
- Corrected hash multiplier calculation in the spatial hash grid by removing an erroneous bit shift operation
- Prevented duplicate tick() calls for CameraEntity instances by adding proper type checking in the entity manager
- Cleaned up unused imports and fixed property access patterns
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Physics/HashGrid.ts | Refactored collision pair deduplication to use triangular matrix indexing, fixed hash multiplier calculation, corrected positionData access, and improved error messages |
| src/Native/Manager.ts | Added type guards to prevent CameraEntity from being ticked twice (once in main loop and once in dedicated camera loop) |
| src/Entity/Misc/Mothership.ts | Removed unused ArenaEntity import, keeping only the used ArenaState export |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [idA, idB] = eidA < eidB ? [eidA, eidB] : [eidB, eidA]; | ||
| const [entA, entB] = eidA < eidB ? [entityA, entityB] : [entityB, entityA]; | ||
|
|
||
| // Triangular matrix index: row * (row - 1) / 2 + col, where row > col |
There was a problem hiding this comment.
The comment says "row * (row - 1) / 2 + col" but the actual formula uses idB and idA. For clarity, consider updating the comment to match the variable names: "Triangular matrix index: idB * (idB - 1) / 2 + idA, where idB > idA".
| // Triangular matrix index: row * (row - 1) / 2 + col, where row > col | |
| // Triangular matrix index: idB * (idB - 1) / 2 + idA, where idB > idA |
| callback: (entityA: ObjectEntity, entityB: ObjectEntity) => void | ||
| ): void { | ||
| if (this.isLocked) throw new Error("HashGrid is locked! Cannot insert entity outside of tick"); | ||
| if (this.isLocked) throw new Error("HashGrid is locked! Cannot forEachCollisionPair() entity outside of tick"); |
There was a problem hiding this comment.
The error message says "Cannot forEachCollisionPair() entity outside of tick" but it should probably be "Cannot call forEachCollisionPair() outside of tick" (without the word "entity") to match the pattern of the other error messages and be grammatically correct.
| if (this.isLocked) throw new Error("HashGrid is locked! Cannot forEachCollisionPair() entity outside of tick"); | |
| if (this.isLocked) throw new Error("HashGrid is locked! Cannot call forEachCollisionPair() outside of tick"); |
|
Should probably work |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Why:
Fixes dup ticks & hash grid
Summarize what's being changed (include any screenshots, code, or other media if available):
Confirm the following: