Skip to content

Commit 9fb16ee

Browse files
obiotclaude
andcommitted
Address Copilot review: guards, bounds clear, DOM listener cleanup
- Container: clear bounds before child aggregation for infinite containers - ImageLayer: throw if parentApp unavailable in onActivateEvent - Input: throw if Application not initialized when enabling pointer events - Application: store DOM event handlers, remove them in destroy() - TMXTileMap: move app resolution inside setViewportBounds branch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 64bb806 commit 9fb16ee

5 files changed

Lines changed: 55 additions & 30 deletions

File tree

packages/melonjs/src/application/application.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,11 @@ export default class Application {
155155

156156
// min update step size
157157
stepSize: number;
158+
159+
// DOM event handlers (stored for cleanup in destroy)
160+
private _onResize?: (e: Event) => void;
161+
private _onOrientationChange?: (e: Event) => void;
162+
private _onScroll?: (e: Event) => void;
158163
updateDelta: number;
159164
lastUpdateStart: number | null;
160165
updateAverageDelta: number;
@@ -313,20 +318,16 @@ export default class Application {
313318
setDefaultGame(this);
314319

315320
// bridge DOM events to the melonJS event system
316-
globalThis.addEventListener("resize", (e) => {
317-
emit(WINDOW_ONRESIZE, e);
318-
});
319-
globalThis.addEventListener("orientationchange", (e) => {
321+
this._onResize = (e: Event) => emit(WINDOW_ONRESIZE, e);
322+
this._onOrientationChange = (e: Event) =>
320323
emit(WINDOW_ONORIENTATION_CHANGE, e);
321-
});
324+
this._onScroll = (e: Event) => emit(WINDOW_ONSCROLL, e);
325+
globalThis.addEventListener("resize", this._onResize);
326+
globalThis.addEventListener("orientationchange", this._onOrientationChange);
322327
if (device.screenOrientation) {
323-
globalThis.screen.orientation.onchange = (e) => {
324-
emit(WINDOW_ONORIENTATION_CHANGE, e);
325-
};
328+
globalThis.screen.orientation.onchange = this._onOrientationChange;
326329
}
327-
globalThis.addEventListener("scroll", (e) => {
328-
emit(WINDOW_ONSCROLL, e);
329-
});
330+
globalThis.addEventListener("scroll", this._onScroll);
330331

331332
// react to resize/orientation changes
332333
on(WINDOW_ONRESIZE, () => {
@@ -516,6 +517,19 @@ export default class Application {
516517
off(STAGE_RESET, this.reset, this);
517518
/* eslint-enable @typescript-eslint/unbound-method */
518519

520+
// remove DOM event listeners
521+
if (this._onResize) {
522+
globalThis.removeEventListener("resize", this._onResize);
523+
globalThis.removeEventListener(
524+
"orientationchange",
525+
this._onOrientationChange!,
526+
);
527+
globalThis.removeEventListener("scroll", this._onScroll!);
528+
if (device.screenOrientation) {
529+
globalThis.screen.orientation.onchange = null;
530+
}
531+
}
532+
519533
// destroy the world and all its children
520534
if (this.world) {
521535
this.world.destroy();

packages/melonjs/src/input/pointerevent.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ function registerEventListener(
138138
*/
139139
function enablePointerEvent(): void {
140140
if (!pointerInitialized) {
141+
if (!_app) {
142+
throw new Error("Pointer events require an initialized Application");
143+
}
141144
// the current pointer area
142145
currentPointer = new Rect(0, 0, 1, 1);
143146

packages/melonjs/src/level/tiled/TMXTileMap.js

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -413,26 +413,26 @@ export default class TMXTileMap {
413413
* callback funtion for the viewport resize event
414414
* @ignore
415415
*/
416-
const app = container.getRootAncestor().app;
417-
418-
function _setBounds(width, height) {
419-
// adjust the viewport bounds if level is smaller
420-
app.viewport.setBounds(
421-
0,
422-
0,
423-
Math.max(levelBounds.width, width),
424-
Math.max(levelBounds.height, height),
425-
);
426-
// center the map if smaller than the current viewport
427-
container.pos.set(
428-
Math.max(0, ~~((width - levelBounds.width) / 2)),
429-
Math.max(0, ~~((height - levelBounds.height) / 2)),
430-
// don't change the container z position if defined
431-
container.pos.z,
432-
);
433-
}
434-
435416
if (setViewportBounds === true) {
417+
const app = container.getRootAncestor().app;
418+
419+
function _setBounds(width, height) {
420+
// adjust the viewport bounds if level is smaller
421+
app.viewport.setBounds(
422+
0,
423+
0,
424+
Math.max(levelBounds.width, width),
425+
Math.max(levelBounds.height, height),
426+
);
427+
// center the map if smaller than the current viewport
428+
container.pos.set(
429+
Math.max(0, ~~((width - levelBounds.width) / 2)),
430+
Math.max(0, ~~((height - levelBounds.height) / 2)),
431+
// don't change the container z position if defined
432+
container.pos.z,
433+
);
434+
}
435+
436436
off(VIEWPORT_ONRESIZE, _setBounds);
437437
// force viewport bounds update
438438
_setBounds(app.viewport.width, app.viewport.height);

packages/melonjs/src/renderable/container.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,9 @@ export default class Container extends Renderable {
570570
if (this.isFinite()) {
571571
// call parent method only when container has finite dimensions
572572
super.updateBounds(absolute);
573+
} else if (this.enableChildBoundsUpdate === true) {
574+
// clear bounds so child aggregation starts fresh
575+
bounds.clear();
573576
}
574577

575578
if (this.enableChildBoundsUpdate === true) {

packages/melonjs/src/renderable/imagelayer.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ export default class ImageLayer extends Sprite {
123123

124124
// called when the layer is added to the game world or a container
125125
onActivateEvent() {
126+
if (!this.parentApp) {
127+
throw new Error(
128+
"ImageLayer requires a parent Application (must be added to an app's world container)",
129+
);
130+
}
126131
const viewport = this.parentApp.viewport;
127132
// set the initial size to match the viewport
128133
this.resize(viewport.width, viewport.height);

0 commit comments

Comments
 (0)