Fix Rect.setSize() not updating bounds (pointer event regression)#1373
Merged
Fix Rect.setSize() not updating bounds (pointer event regression)#1373
Conversation
Rect.setSize() did not call updateBounds(), leaving bounds stale after position or size changes. This broke pointer event broadphase lookups: the QuadTree query rect always reported bounds at (0,0) instead of the actual click position, causing moles at certain positions in the whack-a-mole example to not respond to clicks. Root cause: commit 4d185c9 (July 2024) replaced Rect.setShape(x,y,w,h) with pos.set() + setSize() during the TypeScript conversion, but setSize() never called updateBounds(). The old setShape() updated everything in one call. See #817 for the underlying pos type mismatch. Fix: add updateBounds() call in Rect.setSize(). Tests: add bounds validation tests for Rect.setSize(), pos.set()+setSize(), Rect.copy(), and RoundRect equivalents. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a pointer-event broadphase regression by ensuring Rect.setSize() refreshes the rectangle’s bounds after vertex updates, and adds regression tests to validate bounds updates across Rect and RoundRect.
Changes:
- Call
updateBounds()fromRect.setSize()after updating vertices. - Add unit tests covering bounds updates for
Rect/RoundRectaftersetSize(),pos.set() + setSize(), andcopy(). - Document the fix in the melonJS changelog.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/melonjs/src/geometries/rectangle.ts | Updates Rect.setSize() to refresh bounds after size changes. |
| packages/melonjs/tests/rectangle.spec.ts | Adds regression tests asserting bounds update correctly after setSize() / copy(). |
| packages/melonjs/tests/roundrect.spec.ts | Adds equivalent bounds regression tests for RoundRect. |
| packages/melonjs/CHANGELOG.md | Adds a “Fixed” entry describing the regression and resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -38,6 +38,7 @@ export class Rect extends Polygon { | |||
| this.points[1].set(width, 0); // 1, 0 | |||
| this.points[2].set(width, height); // 1, 1 | |||
| this.points[3].set(0, height); // 0, 1 | |||
There was a problem hiding this comment.
Rect.setSize() mutates points but does not call recalc(). That leaves edges, normals, and indices potentially stale for SAT/collision queries on rectangles that are resized via setSize() (unlike the width/height setters which do recalc() + updateBounds()). Consider calling this.recalc() before this.updateBounds() here to keep the polygon's derived data consistent after resizing.
Suggested change
| this.points[3].set(0, height); // 0, 1 | |
| this.points[3].set(0, height); // 0, 1 | |
| this.recalc(); |
setSize() now calls recalc() before updateBounds(), matching the behavior of the width/height setters. Without this, edges, normals, and indices were stale after setSize(), which could cause incorrect SAT/collision results on resized rectangles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rect.setSize()now callsupdateBounds()after updating vertices4d185c902) where replacingRect.setShape()withpos.set()+setSize()during the TypeScript conversion left bounds stale(0,0)instead of the actual click position, causing objects at certain screen positions to not respond to clicksRoot Cause
The old
Rect.setShape(x, y, w, h)updated position, vertices, and bounds in one call. The replacement split it intopos.set(x, y)+setSize(w, h), butsetSize()never calledupdateBounds(), andPolygon.posis a plainVector2dwith no change observer (unlikeRenderable.poswhich is anObservableVector3d).Test plan
Rect.setSize(),pos.set()+setSize(), andRect.copy()RoundRect🤖 Generated with Claude Code