From 78f3b3b20662fef983eb80244bb66b774de515d6 Mon Sep 17 00:00:00 2001 From: Florian Rival Date: Fri, 6 Sep 2024 14:16:47 +0200 Subject: [PATCH] Fix exception when changing an external tilemap file after it was initially missing --- Extensions/TileMap/JsExtension.js | 110 ++++++++++++++++++------ newIDE/app/src/InstancesEditor/index.js | 83 ++++++++++++------ 2 files changed, 142 insertions(+), 51 deletions(-) diff --git a/Extensions/TileMap/JsExtension.js b/Extensions/TileMap/JsExtension.js index 7cafd4bec419..b57e5ce86f28 100644 --- a/Extensions/TileMap/JsExtension.js +++ b/Extensions/TileMap/JsExtension.js @@ -1616,6 +1616,14 @@ module.exports = { // the Tilemap will properly emit events when hovered/clicked. // By default, this is not implemented in pixi-tilemap. this._pixiObject.containsPoint = (position) => { + if (!this._pixiObject) { + // Ease debugging by throwing now rather than waiting for an exception later. + throw new Error( + 'containsPoint called on a destroyed PIXI object - this object was not properly removed from the PIXI container.' + ); + return; + } + // Turns the world position to the local object coordinates const localPosition = new PIXI.Point(); this._pixiObject.worldTransform.applyInverse(position, localPosition); @@ -1638,21 +1646,32 @@ module.exports = { super.onRemovedFromScene(); // Keep textures because they are shared by all tile maps. this._pixiObject.destroy(false); + + // Not strictly necessary, but helps finding wrong + // handling of this._pixiObject in its container. + this._pixiObject = null; + } + + _replacePixiObject(newPixiObject) { + if (this._pixiObject !== null) + this._pixiContainer.removeChild(this._pixiObject); + this._pixiObject = newPixiObject; + this._pixiContainer.addChild(this._pixiObject); } - onLoadingError() { + _onLoadingError() { this.errorPixiObject = this.errorPixiObject || new PIXI.Sprite(this._pixiResourcesLoader.getInvalidPIXITexture()); - this._pixiContainer.addChild(this.errorPixiObject); - this._pixiObject = this.errorPixiObject; + + this._replacePixiObject(this.errorPixiObject); } - onLoadingSuccess() { + _onLoadingSuccess() { if (this.errorPixiObject) { - this._pixiContainer.removeChild(this.errorPixiObject); + this._replacePixiObject(this.tileMapPixiObject); + this.errorPixiObject = null; - this._pixiObject = this.tileMapPixiObject; } } @@ -1731,7 +1750,7 @@ module.exports = { pako, (tileMap) => { if (!tileMap) { - this.onLoadingError(); + this._onLoadingError(); // _loadTileMapWithCallback already log errors return; } @@ -1750,11 +1769,11 @@ module.exports = { levelIndex, (textureCache) => { if (!textureCache) { - this.onLoadingError(); + this._onLoadingError(); // getOrLoadTextureCache already log warns and errors. return; } - this.onLoadingSuccess(); + this._onLoadingSuccess(); this.width = tileMap.getWidth(); this.height = tileMap.getHeight(); @@ -1920,6 +1939,14 @@ module.exports = { // the Tilemap will properly emit events when hovered/clicked. // By default, this is not implemented in pixi-tilemap. this._pixiObject.containsPoint = (position) => { + if (!this._pixiObject) { + // Ease debugging by throwing now rather than waiting for an exception later. + throw new Error( + 'containsPoint called on a destroyed PIXI object - this object was not properly removed from the PIXI container.' + ); + return; + } + // Turns the world position to the local object coordinates const localPosition = new PIXI.Point(); if (this.tileMapPixiObject.visible) { @@ -1963,21 +1990,32 @@ module.exports = { super.onRemovedFromScene(); // Keep textures because they are shared by all tile maps. this._pixiObject.destroy(false); + + // Not strictly necessary, but helps finding wrong + // handling of this._pixiObject in its container. + this._pixiObject = null; } - onLoadingError() { + _replacePixiObject(newPixiObject) { + if (this._pixiObject !== null) + this._pixiContainer.removeChild(this._pixiObject); + this._pixiObject = newPixiObject; + this._pixiContainer.addChild(this._pixiObject); + } + + _onLoadingError() { this.errorPixiObject = this.errorPixiObject || new PIXI.Sprite(this._pixiResourcesLoader.getInvalidPIXITexture()); - this._pixiContainer.addChild(this.errorPixiObject); - this._pixiObject = this.errorPixiObject; + + this._replacePixiObject(this.errorPixiObject); } - onLoadingSuccess() { + _onLoadingSuccess() { if (this.errorPixiObject) { - this._pixiContainer.removeChild(this.errorPixiObject); + this._replacePixiObject(this.tileMapPixiObject); + this.errorPixiObject = null; - this._pixiObject = this.tileMapPixiObject; } } @@ -2052,7 +2090,7 @@ module.exports = { rowCount, (tileMap) => { if (!tileMap) { - this.onLoadingError(); + this._onLoadingError(); console.error('Could not parse tilemap.'); return; } @@ -2073,7 +2111,7 @@ module.exports = { /** @type {TileMapHelper.TileTextureCache | null} */ textureCache ) => { - this.onLoadingSuccess(); + this._onLoadingSuccess(); if (!this._editableTileMap) return; this.width = this._editableTileMap.getWidth(); @@ -2145,7 +2183,7 @@ module.exports = { /** @type {TileMapHelper.TileTextureCache | null} */ textureCache ) => { - this.onLoadingSuccess(); + this._onLoadingSuccess(); if (!this._editableTileMap) return; this.width = this._editableTileMap.getWidth(); @@ -2253,12 +2291,21 @@ module.exports = { ); this.tileMapPixiObject = new PIXI.Graphics(); + this.tileMapPixiObject._0iAmTheTileMapPixiObject = true; this._pixiObject = this.tileMapPixiObject; // Implement `containsPoint` so that we can set `interactive` to true and // the Tilemap will properly emit events when hovered/clicked. // By default, this is not implemented in pixi-tilemap. this._pixiObject.containsPoint = (position) => { + if (!this._pixiObject) { + // Ease debugging by throwing now rather than waiting for an exception later. + throw new Error( + 'containsPoint called on a destroyed PIXI object - this object was not properly removed from the PIXI container.' + ); + return; + } + // Turns the world position to the local object coordinates const localPosition = new PIXI.Point(); this._pixiObject.worldTransform.applyInverse(position, localPosition); @@ -2281,21 +2328,32 @@ module.exports = { onRemovedFromScene() { super.onRemovedFromScene(); this._pixiObject.destroy(); + + // Not strictly necessary, but helps finding wrong + // handling of this._pixiObject in its container. + this._pixiObject = null; + } + + _replacePixiObject(newPixiObject) { + if (this._pixiObject !== null) + this._pixiContainer.removeChild(this._pixiObject); + this._pixiObject = newPixiObject; + this._pixiContainer.addChild(this._pixiObject); } - onLoadingError() { + _onLoadingError() { this.errorPixiObject = this.errorPixiObject || new PIXI.Sprite(this._pixiResourcesLoader.getInvalidPIXITexture()); - this._pixiContainer.addChild(this.errorPixiObject); - this._pixiObject = this.errorPixiObject; + + this._replacePixiObject(this.errorPixiObject); } - onLoadingSuccess() { + _onLoadingSuccess() { if (this.errorPixiObject) { - this._pixiContainer.removeChild(this.errorPixiObject); + this._replacePixiObject(this.tileMapPixiObject); + this.errorPixiObject = null; - this._pixiObject = this.tileMapPixiObject; } } @@ -2363,11 +2421,11 @@ module.exports = { pako, (tileMap) => { if (!tileMap) { - this.onLoadingError(); + this._onLoadingError(); // _loadTiledMapWithCallback already log errors return; } - this.onLoadingSuccess(); + this._onLoadingSuccess(); this.width = tileMap.getWidth(); this.height = tileMap.getHeight(); diff --git a/newIDE/app/src/InstancesEditor/index.js b/newIDE/app/src/InstancesEditor/index.js index 7fc88d7ae43a..441a6f3d3d27 100644 --- a/newIDE/app/src/InstancesEditor/index.js +++ b/newIDE/app/src/InstancesEditor/index.js @@ -54,6 +54,10 @@ import { import ClickInterceptor from './ClickInterceptor'; import getObjectByName from '../Utils/GetObjectByName'; import { AffineTransformation } from '../Utils/AffineTransformation'; +import { ErrorFallbackComponent } from '../UI/ErrorBoundary'; +import { Trans } from '@lingui/macro'; +import { generateUUID } from 'three/src/math/MathUtils'; + const gd: libGDevelop = global.gd; export const instancesEditorId = 'instances-editor-canvas'; @@ -123,7 +127,14 @@ type Props = {| showObjectInstancesIn3D: boolean, |}; -export default class InstancesEditor extends Component { +type State = {| + renderingError: null | {| + error: Error, + uniqueErrorId: string, + |}, +|}; + +export default class InstancesEditor extends Component { lastContextMenuX = 0; lastContextMenuY = 0; lastCursorX: number | null = null; @@ -162,6 +173,10 @@ export default class InstancesEditor extends Component { hasCursorMovedSinceItIsDown = false; _showObjectInstancesIn3D: boolean = false; + state = { + renderingError: null, + }; + componentDidMount() { // Initialize the PIXI renderer, if possible if (this.canvasArea && !this.pixiRenderer) { @@ -1499,31 +1514,38 @@ export default class InstancesEditor extends Component { if (this._renderingPaused) return; // Avoid killing the CPU by limiting the rendering calls. - if ( - this.fpsLimiter.shouldUpdate() && - !shouldPreventRenderingInstanceEditors() - ) { - this.canvasCursor.render(); - this.grid.render(); - this.highlightedInstance.render(); - this.tileMapPaintingPreview.render(); - this.clickInterceptor.render(); - this.selectedInstances.render(); - this.selectionRectangle.render(); - this.windowBorder.render(); - this.windowMask.render(); - this.statusBar.render(); - this.background.render(); - - this.instancesRenderer.render( - this.pixiRenderer, - this.threeRenderer, - this.viewPosition, - this.uiPixiContainer, - this.backgroundPixiContainer - ); + try { + if ( + this.fpsLimiter.shouldUpdate() && + !shouldPreventRenderingInstanceEditors() + ) { + this.canvasCursor.render(); + this.grid.render(); + this.highlightedInstance.render(); + this.tileMapPaintingPreview.render(); + this.clickInterceptor.render(); + this.selectedInstances.render(); + this.selectionRectangle.render(); + this.windowBorder.render(); + this.windowMask.render(); + this.statusBar.render(); + this.background.render(); + + this.instancesRenderer.render( + this.pixiRenderer, + this.threeRenderer, + this.viewPosition, + this.uiPixiContainer, + this.backgroundPixiContainer + ); + } + this.nextFrame = requestAnimationFrame(this._renderScene); + } catch (error) { + console.error('Exception caught while doing the rendering:', error); + this.setState({ + renderingError: { error, uniqueErrorId: generateUUID() }, + }); } - this.nextFrame = requestAnimationFrame(this._renderScene); }; pauseSceneRendering = () => { @@ -1558,6 +1580,17 @@ export default class InstancesEditor extends Component { render() { if (!this.props.project) return null; + if (this.state.renderingError) { + return ( + Instances editor rendering} + componentStack="[InstancesEditor rendering]" + uniqueErrorId={this.state.renderingError.uniqueErrorId} + /> + ); + } + return ( true}