From 4961acfb89051fa79983f6afa29e26497269cc2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Davy=20H=C3=A9lard?= Date: Mon, 16 Sep 2024 17:50:36 +0200 Subject: [PATCH] Fix hot reload of object variables in object instances --- GDJS/Runtime/debugger-client/hot-reloader.ts | 140 +++++++++------ GDJS/tests/tests/hot-reloader.js | 176 ++++++++++++++++++- 2 files changed, 256 insertions(+), 60 deletions(-) diff --git a/GDJS/Runtime/debugger-client/hot-reloader.ts b/GDJS/Runtime/debugger-client/hot-reloader.ts index 58cff65905d6..141e4e9600d2 100644 --- a/GDJS/Runtime/debugger-client/hot-reloader.ts +++ b/GDJS/Runtime/debugger-client/hot-reloader.ts @@ -28,17 +28,17 @@ namespace gdjs { this._runtimeGame = runtimeGame; } - static groupByPersistentUuid< + static indexByPersistentUuid< ObjectWithPersistentId extends { persistentUuid: string | null } >( objectsWithPersistentId: ObjectWithPersistentId[] - ): Record { + ): Map { return objectsWithPersistentId.reduce(function (objectsMap, object) { if (object.persistentUuid) { - objectsMap[object.persistentUuid] = object; + objectsMap.set(object.persistentUuid, object); } return objectsMap; - }, {}); + }, new Map()); } static indexByName( @@ -972,18 +972,9 @@ namespace gdjs { hotReloadSucceeded; }); - // Don't update the variables, behaviors and effects for each runtime object to avoid + // Don't update behaviors and effects for each runtime object to avoid // doing the check for differences for every single object. - // Update variables - runtimeObjects.forEach((runtimeObject) => { - this._hotReloadVariablesContainer( - oldObjectData.variables as Required[], - newObjectData.variables as Required[], - runtimeObject.getVariables() - ); - }); - // Update behaviors this._hotReloadRuntimeObjectsBehaviors( oldObjectData.behaviors, @@ -1353,23 +1344,19 @@ namespace gdjs { runtimeInstanceContainer: gdjs.RuntimeInstanceContainer ): void { const runtimeObjects = runtimeInstanceContainer.getAdhocListOfAllInstances(); - const groupedOldInstances: { - [key: number]: InstanceData; - } = HotReloader.groupByPersistentUuid(oldInstances); - const groupedNewInstances: { - [key: number]: InstanceData; - } = HotReloader.groupByPersistentUuid(newInstances); - const groupedRuntimeObjects: { - [key: number]: gdjs.RuntimeObject; - } = HotReloader.groupByPersistentUuid(runtimeObjects); + const oldInstanceByUuid = HotReloader.indexByPersistentUuid(oldInstances); + const newInstanceByUuid = HotReloader.indexByPersistentUuid(newInstances); + const runtimeObjectByUuid = HotReloader.indexByPersistentUuid( + runtimeObjects + ); const oldObjectsMap = HotReloader.indexByName(oldObjects); const newObjectsMap = HotReloader.indexByName(newObjects); - for (const persistentUuid in groupedOldInstances) { - const oldInstance = groupedOldInstances[persistentUuid]; - const newInstance = groupedNewInstances[persistentUuid]; - const runtimeObject = groupedRuntimeObjects[persistentUuid]; + for (const persistentUuid of oldInstanceByUuid.keys()) { + const oldInstance = oldInstanceByUuid.get(persistentUuid); + const newInstance = newInstanceByUuid.get(persistentUuid); + const runtimeObject = runtimeObjectByUuid.get(persistentUuid); if ( oldInstance && @@ -1383,16 +1370,21 @@ namespace gdjs { } } - for (const persistentUuid in groupedRuntimeObjects) { - const runtimeObject = groupedRuntimeObjects[persistentUuid]; + for (const runtimeObject of runtimeObjects) { const oldObjectData = oldObjectsMap.get(runtimeObject.getName()); const newObjectData = newObjectsMap.get(runtimeObject.getName()); if (!runtimeObject || !oldObjectData || !newObjectData) { // New objects or deleted objects can't have instances to hot-reload. continue; } - const oldInstance = groupedOldInstances[persistentUuid]; - const newInstance = groupedNewInstances[persistentUuid]; + const oldInstance = oldInstanceByUuid.get( + // @ts-ignore Private attribute + runtimeObject.persistentUuid + ); + const newInstance = newInstanceByUuid.get( + // @ts-ignore Private attribute + runtimeObject.persistentUuid + ); if (oldInstance && newInstance) { // Instance was not deleted nor created, maybe modified (or not): this._hotReloadRuntimeInstance( @@ -1405,33 +1397,44 @@ namespace gdjs { newInstance, runtimeObject ); - } else if (runtimeObject instanceof gdjs.CustomRuntimeObject) { - const childrenInstanceContainer = runtimeObject.getChildrenContainer(); - - // The `objects` attribute is already resolved by `resolveCustomObjectConfigurations()`. - const oldCustomObjectData = oldObjectData as ObjectData & - CustomObjectConfiguration & - InstanceContainerData; - const newCustomObjectData = newObjectData as ObjectData & - CustomObjectConfiguration & - InstanceContainerData; - - // Reload the content of custom objects that were created at runtime. - this._hotReloadRuntimeInstanceContainer( - oldProjectData, - newProjectData, - oldCustomObjectData, - newCustomObjectData, - changedRuntimeBehaviors, - childrenInstanceContainer + } else { + // Reload objects that were created at runtime. + + // Update variables + this._hotReloadVariablesContainer( + oldObjectData.variables, + newObjectData.variables, + runtimeObject.getVariables() ); + + if (runtimeObject instanceof gdjs.CustomRuntimeObject) { + const childrenInstanceContainer = runtimeObject.getChildrenContainer(); + + // The `objects` attribute is already resolved by `resolveCustomObjectConfigurations()`. + const oldCustomObjectData = oldObjectData as ObjectData & + CustomObjectConfiguration & + InstanceContainerData; + const newCustomObjectData = newObjectData as ObjectData & + CustomObjectConfiguration & + InstanceContainerData; + + // Reload the content of custom objects that were created at runtime. + this._hotReloadRuntimeInstanceContainer( + oldProjectData, + newProjectData, + oldCustomObjectData, + newCustomObjectData, + changedRuntimeBehaviors, + childrenInstanceContainer + ); + } } } - for (const persistentUuid in groupedNewInstances) { - const oldInstance = groupedOldInstances[persistentUuid]; - const newInstance = groupedNewInstances[persistentUuid]; - const runtimeObject = groupedRuntimeObjects[persistentUuid]; + for (const persistentUuid of newInstanceByUuid.keys()) { + const oldInstance = oldInstanceByUuid.get(persistentUuid); + const newInstance = newInstanceByUuid.get(persistentUuid); + const runtimeObject = runtimeObjectByUuid.get(persistentUuid); if ( newInstance && (!oldInstance || oldInstance.name !== newInstance.name) && @@ -1577,8 +1580,14 @@ namespace gdjs { // Update variables this._hotReloadVariablesContainer( - oldInstance.initialVariables as Required[], - newInstance.initialVariables as Required[], + this._mergeObjectVariablesData( + oldObjectData.variables, + oldInstance.initialVariables + ), + this._mergeObjectVariablesData( + newObjectData.variables, + newInstance.initialVariables + ), runtimeObject.getVariables() ); @@ -1618,6 +1627,25 @@ namespace gdjs { } } + _mergeObjectVariablesData( + objectVariablesData: RootVariableData[], + instanceVariablesData: RootVariableData[] + ): RootVariableData[] { + if (instanceVariablesData.length === 0) { + return objectVariablesData; + } + const variablesData = [...objectVariablesData]; + for (const instanceVariableData of instanceVariablesData) { + const index = variablesData.indexOf( + (variableData) => variableData.name === instanceVariableData.name + ); + if (index >= 0) { + variablesData[index] = instanceVariableData; + } + } + return variablesData; + } + /** * Deep check equality between the two objects/arrays/primitives. * diff --git a/GDJS/tests/tests/hot-reloader.js b/GDJS/tests/tests/hot-reloader.js index 5569acf0ff66..26b6c32017a1 100644 --- a/GDJS/tests/tests/hot-reloader.js +++ b/GDJS/tests/tests/hot-reloader.js @@ -117,15 +117,15 @@ describe('gdjs.HotReloader._hotReloadRuntimeGame', () => { /** * Create and return a minimum working sprite object data. * @internal - * @param {{name: string, image: string}} data + * @param {{name: string, image?: string, variables?: Array}} data * @returns {gdjs.SpriteObjectData} */ - const createSpriteData = ({ name, image }) => { + const createSpriteData = ({ name, image, variables }) => { return { type: 'Sprite', name, behaviors: [], - variables: [], + variables: variables || [], effects: [], animations: [ { @@ -144,7 +144,7 @@ describe('gdjs.HotReloader._hotReloadRuntimeGame', () => { points: [], hasCustomCollisionMask: false, customCollisionMask: [], - image, + image: image === undefined ? 'MyImageResource' : image, }, ], timeBetweenFrames: 0, @@ -453,6 +453,174 @@ describe('gdjs.HotReloader._hotReloadRuntimeGame', () => { expect(getSpriteCurrentFrameImage(instances[0])).to.be('ResourceB'); expect(getSpriteCurrentFrameImage(instances[1])).to.be('ResourceB'); }); + + it('can change variable values of initial instances at hot-reload', async () => { + const oldProjectData = createProjectData({ + layouts: [ + createSceneData({ + instances: [{ persistentUuid: '1' }], + objects: [ + createSpriteData({ + name: 'MyObject', + variables: [ + { + name: 'MyVariable1', + value: '123', + type: 'number', + }, + { + name: 'MyVariable2', + value: '456', + type: 'number', + }, + ], + }), + ], + }), + ], + }); + const runtimeGame = new gdjs.RuntimeGame(oldProjectData); + const hotReloader = new gdjs.HotReloader(runtimeGame); + await runtimeGame.loadFirstAssetsAndStartBackgroundLoading( + 'Scene1', + () => {} + ); + runtimeGame._sceneStack.push('Scene1'); + const scene = runtimeGame.getSceneStack().getCurrentScene(); + if (!scene) throw new Error("Couldn't set a current scene for testing."); + + const instances = scene.getInstancesOf('MyObject'); + expect(instances.length).to.be(1); + const variablesContainer = instances[0].getVariables(); + // The variable values are changed by events. + variablesContainer.get('MyVariable1').setNumber(111); + variablesContainer.get('MyVariable2').setNumber(222); + + const newProjectData = createProjectData({ + layouts: [ + createSceneData({ + instances: [{ persistentUuid: '1' }], + objects: [ + createSpriteData({ + name: 'MyObject', + variables: [ + { + name: 'MyVariable1', + value: '123', + type: 'number', + }, + { + name: 'MyVariable2', + value: '789', + type: 'number', + }, + ], + }), + ], + }), + ], + }); + + await hotReloader._hotReloadRuntimeGame( + oldProjectData, + newProjectData, + [], + runtimeGame + ); + + // The current value is kept. + expect(variablesContainer.has('MyVariable1')).to.be(true); + expect(variablesContainer.get('MyVariable1').getAsNumber()).to.be(111); + expect(variablesContainer.getFromIndex(0).getAsNumber()).to.be(111); + // The current value is overridden. + expect(variablesContainer.has('MyVariable2')).to.be(true); + expect(variablesContainer.get('MyVariable2').getAsNumber()).to.be(789); + expect(variablesContainer.getFromIndex(1).getAsNumber()).to.be(789); + }); + + it('can change variable values of instances created at runtime at hot-reload', async () => { + const oldProjectData = createProjectData({ + layouts: [ + createSceneData({ + instances: [], + objects: [ + createSpriteData({ + name: 'MyObject', + variables: [ + { + name: 'MyVariable1', + value: '123', + type: 'number', + }, + { + name: 'MyVariable2', + value: '456', + type: 'number', + }, + ], + }), + ], + }), + ], + }); + const runtimeGame = new gdjs.RuntimeGame(oldProjectData); + const hotReloader = new gdjs.HotReloader(runtimeGame); + await runtimeGame.loadFirstAssetsAndStartBackgroundLoading( + 'Scene1', + () => {} + ); + runtimeGame._sceneStack.push('Scene1'); + const scene = runtimeGame.getSceneStack().getCurrentScene(); + if (!scene) throw new Error("Couldn't set a current scene for testing."); + + const instances = scene.getInstancesOf('MyObject'); + const instance = scene.createObject('MyObject'); + const variablesContainer = instance.getVariables(); + // The variable values are changed by events. + variablesContainer.get('MyVariable1').setNumber(111); + variablesContainer.get('MyVariable2').setNumber(222); + + const newProjectData = createProjectData({ + layouts: [ + createSceneData({ + instances: [], + objects: [ + createSpriteData({ + name: 'MyObject', + variables: [ + { + name: 'MyVariable1', + value: '123', + type: 'number', + }, + { + name: 'MyVariable2', + value: '789', + type: 'number', + }, + ], + }), + ], + }), + ], + }); + + await hotReloader._hotReloadRuntimeGame( + oldProjectData, + newProjectData, + [], + runtimeGame + ); + + // The current value is kept. + expect(variablesContainer.has('MyVariable1')).to.be(true); + expect(variablesContainer.get('MyVariable1').getAsNumber()).to.be(111); + expect(variablesContainer.getFromIndex(0).getAsNumber()).to.be(111); + // The current value is overridden. + expect(variablesContainer.has('MyVariable2')).to.be(true); + expect(variablesContainer.get('MyVariable2').getAsNumber()).to.be(789); + expect(variablesContainer.getFromIndex(1).getAsNumber()).to.be(789); + }); }); describe('gdjs.HotReloader._hotReloadVariablesContainer', () => {