From 5b9693b1f2302a11ef09ac302e793abe0a31d7b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Thu, 30 Jul 2026 16:53:06 +0200 Subject: [PATCH 1/3] Allow undo/redo after a change in the instance panel --- newIDE/app/src/SceneEditor/index.js | 40 ++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/newIDE/app/src/SceneEditor/index.js b/newIDE/app/src/SceneEditor/index.js index b4e23625c983..63f2d8fb1362 100644 --- a/newIDE/app/src/SceneEditor/index.js +++ b/newIDE/app/src/SceneEditor/index.js @@ -460,6 +460,9 @@ export default class SceneEditor extends React.Component { this.unregisterDebuggerCallback(); this.unregisterDebuggerCallback = null; } + // Cancelled, not flushed: the history lives in the state of this + // component, so there is nothing left to save it to. + this._saveInstancesModificationsToHistoryDebounced.cancel(); } onEditorReloaded() { @@ -1164,6 +1167,22 @@ export default class SceneEditor extends React.Component { { leading: false, trailing: true } ): any); + /** + * Return the history, with any instance modification pending a debounced + * save (see `_onInstancesModified`) committed to it. Must be used by + * undo/redo: an uncommitted modification would otherwise be reverted + * without being redoable (`undo` restores the last *saved* snapshot). + */ + _getHistoryWithPendingInstancesModificationsSaved = (): HistoryState => { + this._saveInstancesModificationsToHistoryDebounced.cancel(); + if (!this._hasPendingInstancesModificationsToSave) { + return this.state.history; + } + + this._hasPendingInstancesModificationsToSave = false; + return saveToHistory(this.state.history, this.props.initialInstances); + }; + undo = () => { // /!\ Drop the selection to avoid keeping any references to deleted instances. // This could be avoided if the selection used something like UUID to address instances. @@ -1171,7 +1190,7 @@ export default class SceneEditor extends React.Component { this.setState( { history: undo( - this.state.history, + this._getHistoryWithPendingInstancesModificationsSaved(), this.props.initialInstances, this.props.project ), @@ -1194,7 +1213,7 @@ export default class SceneEditor extends React.Component { this.setState( { history: redo( - this.state.history, + this._getHistoryWithPendingInstancesModificationsSaved(), this.props.initialInstances, this.props.project ), @@ -1502,9 +1521,24 @@ export default class SceneEditor extends React.Component { _onInstancesModified = (instances: Array) => { this._sendUpdatedInstances(instances); this.forceUpdate(); - //TODO: Save for redo with debounce (and cancel on unmount) + // Save the modification for undo/redo, debounced so a rapid series of + // changes (like typing a position digit by digit) makes a single + // undoable step. + this._hasPendingInstancesModificationsToSave = true; + this._saveInstancesModificationsToHistoryDebounced(); }; + _hasPendingInstancesModificationsToSave = false; + + // $FlowFixMe[missing-local-annot] + _saveInstancesModificationsToHistoryDebounced = (debounce(() => { + if (!this._hasPendingInstancesModificationsToSave) return; + this._hasPendingInstancesModificationsToSave = false; + this.setState({ + history: saveToHistory(this.state.history, this.props.initialInstances), + }); + }, 500): any); + _sendUpdatedInstances = (instances: Array) => { const { previewDebuggerServer } = this.props; if (!previewDebuggerServer) return; From cc195ae12f2b52486f0c122aecaad5629aa1a822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:27:31 +0200 Subject: [PATCH 2/3] Wip improve undo/redo in the editor --- .../app/src/EmbeddedGame/EmbeddedGameFrame.js | 16 + .../InstancesRenderer/index.js | 78 ++- newIDE/app/src/InstancesEditor/index.js | 84 +++ .../CompactObjectPropertiesEditor/index.js | 6 +- .../CompactScenePropertiesEditor/index.js | 6 +- .../DialogUndoShortcutPropagation.spec.js | 104 ++++ .../src/SceneEditor/EditorsDisplay.flow.js | 4 + ...stanceOrObjectPropertiesEditorContainer.js | 3 + .../SceneEditor/MosaicEditorsDisplay/index.js | 8 + .../SwipeableDrawerEditorsDisplay/index.js | 10 + newIDE/app/src/SceneEditor/index.js | 545 +++++++++++++++--- newIDE/app/src/Utils/CompositeHistory.spec.js | 129 +++++ newIDE/app/src/Utils/History.js | 175 ++++++ newIDE/app/src/Utils/InstancesSnapshotDiff.js | 53 ++ .../src/Utils/InstancesSnapshotDiff.spec.js | 78 +++ 15 files changed, 1198 insertions(+), 101 deletions(-) create mode 100644 newIDE/app/src/SceneEditor/DialogUndoShortcutPropagation.spec.js create mode 100644 newIDE/app/src/Utils/CompositeHistory.spec.js create mode 100644 newIDE/app/src/Utils/InstancesSnapshotDiff.js create mode 100644 newIDE/app/src/Utils/InstancesSnapshotDiff.spec.js diff --git a/newIDE/app/src/EmbeddedGame/EmbeddedGameFrame.js b/newIDE/app/src/EmbeddedGame/EmbeddedGameFrame.js index 7bf47a255ea4..7ba7586fb04d 100644 --- a/newIDE/app/src/EmbeddedGame/EmbeddedGameFrame.js +++ b/newIDE/app/src/EmbeddedGame/EmbeddedGameFrame.js @@ -110,6 +110,7 @@ let onSetCameraState: let onChangeViewPosition: | null | ((command: ChangeViewPositionCommand) => void) = null; +let onFocusEmbeddedGameFrame: null | (() => boolean) = null; export const setEmbeddedGameFramePreviewLocation = ({ previewIndexHtmlLocation, @@ -164,6 +165,15 @@ export const changeViewPosition = (command: ChangeViewPositionCommand) => { onChangeViewPosition(command); }; +/** + * Give the keyboard focus to the embedded game frame, so the in-game + * editor shortcuts work. Return false if there is no frame to focus. + */ +export const focusEmbeddedGameFrame = (): boolean => { + if (!onFocusEmbeddedGameFrame) return false; + return onFocusEmbeddedGameFrame(); +}; + const logSwitchingInfo = ({ editorId, sceneName, @@ -492,6 +502,12 @@ export const EmbeddedGameFrame = ({ }); }); }; + onFocusEmbeddedGameFrame = () => { + const iframe = iframeRef.current; + if (!iframe || !iframe.contentWindow) return false; + iframe.contentWindow.focus(); + return true; + }; }, [ previewDebuggerServer, diff --git a/newIDE/app/src/InstancesEditor/InstancesRenderer/index.js b/newIDE/app/src/InstancesEditor/InstancesRenderer/index.js index 74a2cad7861c..6b1a0cc09734 100644 --- a/newIDE/app/src/InstancesEditor/InstancesRenderer/index.js +++ b/newIDE/app/src/InstancesEditor/InstancesRenderer/index.js @@ -192,6 +192,57 @@ export default class InstancesRenderer { return this._basicProfilingCounters; } + _getOrCreateLayerRenderer( + layer: gdLayer, + // $FlowFixMe[value-as-type] + pixiRenderer: PIXI.Renderer + ): LayerRenderer { + const layerName = layer.getName(); + let layerRenderer = this.layersRenderers[layerName]; + if (!layerRenderer) { + this.layersRenderers[layerName] = layerRenderer = new LayerRenderer({ + project: this.project, + globalObjectsContainer: this.globalObjectsContainer, + objectsContainer: this.objectsContainer, + instances: this.instances, + viewPosition: this.viewPosition, + layer: layer, + onInstanceClicked: this.onInstanceClicked, + onInstanceRightClicked: this.onInstanceRightClicked, + onInstanceDoubleClicked: this.onInstanceDoubleClicked, + onOverInstance: this.onOverInstance, + onOutInstance: this.onOutInstance, + onMoveInstance: this.onMoveInstance, + onMoveInstanceEnd: this.onMoveInstanceEnd, + onDownInstance: this.onDownInstance, + onUpInstance: this.onUpInstance, + pixiRenderer: pixiRenderer, + showObjectInstancesIn3D: this._showObjectInstancesIn3D, + }); + this.pixiContainer.addChild(layerRenderer.getPixiContainer()); + } + return layerRenderer; + } + + /** + * Create the renderers of the layers that don't have one yet. They are + * normally created lazily at the first render: call this to measure + * instances (see `getInstanceMeasurer`) before it happened - for example + * right after the renderers were remounted, as the measurer falls back + * to a zero-sized rectangle at the instance origin without them. + */ + ensureLayerRenderersExist( + // $FlowFixMe[value-as-type] + pixiRenderer: PIXI.Renderer + ) { + for (let i = 0; i < this.layersContainer.getLayersCount(); i++) { + this._getOrCreateLayerRenderer( + this.layersContainer.getLayerAt(i), + pixiRenderer + ); + } + } + render( // $FlowFixMe[value-as-type] pixiRenderer: PIXI.Renderer, @@ -232,31 +283,8 @@ export default class InstancesRenderer { for (let i = 0; i < this.layersContainer.getLayersCount(); i++) { const layer = this.layersContainer.getLayerAt(i); - const layerName = layer.getName(); - - let layerRenderer = this.layersRenderers[layerName]; - if (!layerRenderer) { - this.layersRenderers[layerName] = layerRenderer = new LayerRenderer({ - project: this.project, - globalObjectsContainer: this.globalObjectsContainer, - objectsContainer: this.objectsContainer, - instances: this.instances, - viewPosition: this.viewPosition, - layer: layer, - onInstanceClicked: this.onInstanceClicked, - onInstanceRightClicked: this.onInstanceRightClicked, - onInstanceDoubleClicked: this.onInstanceDoubleClicked, - onOverInstance: this.onOverInstance, - onOutInstance: this.onOutInstance, - onMoveInstance: this.onMoveInstance, - onMoveInstanceEnd: this.onMoveInstanceEnd, - onDownInstance: this.onDownInstance, - onUpInstance: this.onUpInstance, - pixiRenderer: pixiRenderer, - showObjectInstancesIn3D: this._showObjectInstancesIn3D, - }); - this.pixiContainer.addChild(layerRenderer.getPixiContainer()); - } + + const layerRenderer = this._getOrCreateLayerRenderer(layer, pixiRenderer); // /!\ Objects representing layers can be deleted at any moment and replaced // by new one, for example when two layers are swapped. diff --git a/newIDE/app/src/InstancesEditor/index.js b/newIDE/app/src/InstancesEditor/index.js index 544a2a9e9ec1..b35548a1bdce 100644 --- a/newIDE/app/src/InstancesEditor/index.js +++ b/newIDE/app/src/InstancesEditor/index.js @@ -1771,6 +1771,12 @@ export default class InstancesEditor extends Component { ) => { if (instances.length === 0) return; + // Centering can be requested before the first render created the layer + // renderers (for example right after an undo/redo remounted them), in + // which case the measured rectangle would collapse to the instance + // origin (its top-left corner most of the time) instead of its center. + this.instancesRenderer.ensureLayerRenderersExist(this.pixiRenderer); + const instanceMeasurer = this.instancesRenderer.getInstanceMeasurer(); let lastInstanceRectangle = instanceMeasurer.getInstanceAABB( instances[instances.length - 1], @@ -1780,6 +1786,84 @@ export default class InstancesEditor extends Component { if (offset) this.scrollBy(offset[0], offset[1]); }; + /** + * Scroll the view by the smallest amount to make the last instance + * visible (with a margin), so the view does a small jump towards it - + * keeping the direction of the movement - rather than re-centering on it. + */ + scrollViewToLastInstance = (instances: Array) => { + if (instances.length === 0) return; + + // See `centerViewOnLastInstance` about why this is needed. + this.instancesRenderer.ensureLayerRenderersExist(this.pixiRenderer); + const aabb = this.instancesRenderer + .getInstanceMeasurer() + .getInstanceAABB(instances[instances.length - 1], new Rectangle()); + this._scrollViewMinimallyToRectangle(aabb); + }; + + /** + * Scroll the view by the smallest amount to make the given point visible + * (with a margin). + */ + scrollViewToPoint = (x: number, y: number) => { + this._scrollViewMinimallyToRectangle(new Rectangle(x, y, x, y)); + }; + + _scrollViewMinimallyToRectangle = (rectangle: Rectangle) => { + const [viewLeft, viewTop] = this.viewPosition.toSceneCoordinates(0, 0); + const [viewRight, viewBottom] = this.viewPosition.toSceneCoordinates( + this.viewPosition.getWidth(), + this.viewPosition.getHeight() + ); + // Keep a margin so the revealed target is not stuck to the view border. + const marginX = (viewRight - viewLeft) * 0.1; + const marginY = (viewBottom - viewTop) * 0.1; + + // Only scroll on an axis where the rectangle is completely out of the + // view: an axis where it's already (even partly) visible must not + // move, so the jump keeps the direction of the target. + let scrollX = 0; + if (rectangle.right < viewLeft) { + scrollX = rectangle.left - (viewLeft + marginX); + } else if (rectangle.left > viewRight) { + scrollX = rectangle.right - (viewRight - marginX); + } + let scrollY = 0; + if (rectangle.bottom < viewTop) { + scrollY = rectangle.top - (viewTop + marginY); + } else if (rectangle.top > viewBottom) { + scrollY = rectangle.bottom - (viewBottom - marginY); + } + if (scrollX || scrollY) this.scrollBy(scrollX, scrollY); + }; + + /** + * Return true if any part of the instance is currently visible in the + * viewport. + */ + isInstanceVisibleInViewport = (instance: gdInitialInstance): boolean => { + // The layer renderers (needed to measure instances) are created lazily + // at the first render: ensure they exist (this can be called right + // after a remount, before any render). + this.instancesRenderer.ensureLayerRenderersExist(this.pixiRenderer); + const aabb = this.instancesRenderer + .getInstanceMeasurer() + .getInstanceAABB(instance, new Rectangle()); + + const viewTopLeft = this.viewPosition.toSceneCoordinates(0, 0); + const viewBottomRight = this.viewPosition.toSceneCoordinates( + this.viewPosition.getWidth(), + this.viewPosition.getHeight() + ); + return ( + aabb.right >= viewTopLeft[0] && + aabb.left <= viewBottomRight[0] && + aabb.bottom >= viewTopLeft[1] && + aabb.top <= viewBottomRight[1] + ); + }; + getLastContextMenuSceneCoordinates = (): any => { return this.viewPosition.toSceneCoordinates( this.lastContextMenuX, diff --git a/newIDE/app/src/ObjectEditor/CompactObjectPropertiesEditor/index.js b/newIDE/app/src/ObjectEditor/CompactObjectPropertiesEditor/index.js index 21cd63d27141..35dec4804090 100644 --- a/newIDE/app/src/ObjectEditor/CompactObjectPropertiesEditor/index.js +++ b/newIDE/app/src/ObjectEditor/CompactObjectPropertiesEditor/index.js @@ -246,7 +246,6 @@ export const CompactObjectPropertiesEditor = ({ projectScopedContainersAccessor, unsavedChanges, i18n, - historyHandler, objects, onEditObject, onObjectsModified, @@ -871,7 +870,10 @@ export const CompactObjectPropertiesEditor = ({ ) : [] } - historyHandler={historyHandler} + // Don't give the scene editor `historyHandler`: it tracks + // the scene (instances, layers...), not the objects — it + // would record no-op undo steps. Without it, the list + // falls back on its own local history. onVariablesUpdated={onVariablesUpdated} toolbarIconStyle={styles.icon} compactEmptyPlaceholderText={ diff --git a/newIDE/app/src/SceneEditor/CompactScenePropertiesEditor/index.js b/newIDE/app/src/SceneEditor/CompactScenePropertiesEditor/index.js index cfb78f4b914b..f6ef722287b4 100644 --- a/newIDE/app/src/SceneEditor/CompactScenePropertiesEditor/index.js +++ b/newIDE/app/src/SceneEditor/CompactScenePropertiesEditor/index.js @@ -75,6 +75,7 @@ type Props = {| unsavedChanges?: ?UnsavedChanges, i18n: I18nType, historyHandler?: HistoryHandler, + onScenePropertiesModified?: () => void, |}; export const CompactScenePropertiesEditor = ({ @@ -87,6 +88,7 @@ export const CompactScenePropertiesEditor = ({ unsavedChanges, i18n, historyHandler, + onScenePropertiesModified, }: Props): React.Node => { const forceUpdate = useForceUpdate(); const variablesListRef = React.useRef(null); @@ -201,9 +203,7 @@ export const CompactScenePropertiesEditor = ({ project={project} schema={propertiesSchema} instances={[scene]} - onInstancesModified={() => { - // TODO: undo/redo? - }} + onInstancesModified={onScenePropertiesModified || noop} resourceManagementProps={resourceManagementProps} placeholder="" onRefreshAllFields={forceRecomputeSchema} diff --git a/newIDE/app/src/SceneEditor/DialogUndoShortcutPropagation.spec.js b/newIDE/app/src/SceneEditor/DialogUndoShortcutPropagation.spec.js new file mode 100644 index 000000000000..62bcec6f8c93 --- /dev/null +++ b/newIDE/app/src/SceneEditor/DialogUndoShortcutPropagation.spec.js @@ -0,0 +1,104 @@ +/** + * @jest-environment jsdom + * @flow + */ +import * as React from 'react'; +import { createRoot } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import Dialog from '@material-ui/core/Dialog'; + +// The scene editor listens to `keydown` on its root div to catch undo/redo +// shortcuts (see `_onKeyDownInEditor` in `SceneEditor/index.js`). Dialogs are +// rendered in portals: their DOM lives outside of the root div, but React +// synthetic events propagate through the REACT tree, not the DOM tree - so a +// shortcut pressed inside a dialog DOES reach the root div handler. +// These tests document this behavior and the DOM containment check +// (`container.contains(event.target)`) used by `_onKeyDownInEditor` to +// distinguish the two cases. +describe('keydown propagation from a dialog to a parent onKeyDown', () => { + let container: HTMLDivElement; + let root; + + beforeEach(() => { + container = document.createElement('div'); + if (!document.body) throw new Error('No document body'); + document.body.appendChild(container); + }); + + afterEach(() => { + act(() => { + if (root) root.unmount(); + }); + container.remove(); + }); + + const dispatchCtrlZOn = (element: HTMLElement) => { + act(() => { + element.focus(); + element.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'z', + ctrlKey: true, + bubbles: true, + cancelable: true, + }) + ); + }); + }; + + it('reaches the parent handler from an element inside the div, which contains the event target', () => { + const receivedEvents = []; + const onKeyDown = (event: SyntheticKeyboardEvent) => + receivedEvents.push({ + containsTarget: + event.target instanceof Node && + event.currentTarget.contains(event.target), + }); + act(() => { + root = createRoot(container); + root.render( +
+ +
+ ); + }); + + const button = document.getElementById('in-div-button'); + if (!button) throw new Error('Button not found'); + dispatchCtrlZOn(button); + + expect(receivedEvents).toEqual([{ containsTarget: true }]); + }); + + it('reaches the parent handler from inside a portaled dialog, but the div does NOT contain the event target', () => { + const receivedEvents = []; + const onKeyDown = (event: SyntheticKeyboardEvent) => + receivedEvents.push({ + containsTarget: + event.target instanceof Node && + event.currentTarget.contains(event.target), + }); + act(() => { + root = createRoot(container); + root.render( +
+ + + +
+ ); + }); + + const button = document.getElementById('dialog-button'); + if (!button) throw new Error('Dialog button not found'); + dispatchCtrlZOn(button); + + // The synthetic event DOES reach the parent handler (React tree + // propagation): without a guard, shortcuts pressed in a dialog would + // wrongly trigger the editor ones. + expect(receivedEvents.length).toBe(1); + // ...but the DOM containment check tells the two cases apart (the + // dialog DOM is in a portal, outside of the div). + expect(receivedEvents[0].containsTarget).toBe(false); + }); +}); diff --git a/newIDE/app/src/SceneEditor/EditorsDisplay.flow.js b/newIDE/app/src/SceneEditor/EditorsDisplay.flow.js index 50ab0937ddec..92c02c231b82 100644 --- a/newIDE/app/src/SceneEditor/EditorsDisplay.flow.js +++ b/newIDE/app/src/SceneEditor/EditorsDisplay.flow.js @@ -44,6 +44,7 @@ export type SceneEditorsDisplayProps = {| targetPosition?: 'center' | 'upperCenter' ) => void, onInstancesModified?: (Array) => void, + onScenePropertiesModified?: () => void, editInstanceVariables: (instance: ?gdInitialInstance) => void, editObjectByName: ({ objectName: string, @@ -198,6 +199,9 @@ export type SceneEditorsDisplayInterface = {| Array, offset?: ?[number, number] ) => void, + isInstanceVisibleInViewport: gdInitialInstance => boolean, + scrollViewToLastInstance: (Array) => void, + scrollViewToPoint: (x: number, y: number) => void, getLastCursorSceneCoordinates: () => [number, number], getLastContextMenuSceneCoordinates: () => [number, number], getViewPosition: () => ?ViewPosition, diff --git a/newIDE/app/src/SceneEditor/InstanceOrObjectPropertiesEditorContainer.js b/newIDE/app/src/SceneEditor/InstanceOrObjectPropertiesEditorContainer.js index 0dd71f963dce..2e3a67371a2c 100644 --- a/newIDE/app/src/SceneEditor/InstanceOrObjectPropertiesEditorContainer.js +++ b/newIDE/app/src/SceneEditor/InstanceOrObjectPropertiesEditorContainer.js @@ -69,6 +69,7 @@ type Props = {| instances: Array, editObjectInPropertiesPanel: (objectName: string) => void, onInstancesModified?: (Array) => void, + onScenePropertiesModified?: () => void, onGetInstanceSize: gdInitialInstance => [number, number, number], editInstanceVariables: gdInitialInstance => void, tileMapTileSelection: ?TileMapTileSelection, @@ -158,6 +159,7 @@ export const InstanceOrObjectPropertiesEditorContainer: React.ComponentType<{ instances, editObjectInPropertiesPanel, onInstancesModified, + onScenePropertiesModified, onGetInstanceSize, editInstanceVariables, tileMapTileSelection, @@ -308,6 +310,7 @@ export const InstanceOrObjectPropertiesEditorContainer: React.ComponentType<{ i18n={i18n} onBackgroundColorChanged={onBackgroundColorChanged} openSceneVariables={openSceneVariables} + onScenePropertiesModified={onScenePropertiesModified} /> ) : null} diff --git a/newIDE/app/src/SceneEditor/MosaicEditorsDisplay/index.js b/newIDE/app/src/SceneEditor/MosaicEditorsDisplay/index.js index 51141fa4ad21..fd8a81b96b9a 100644 --- a/newIDE/app/src/SceneEditor/MosaicEditorsDisplay/index.js +++ b/newIDE/app/src/SceneEditor/MosaicEditorsDisplay/index.js @@ -231,6 +231,13 @@ const MosaicEditorsDisplay: React.ComponentType<{ centerViewOnLastInstance: editor ? editor.centerViewOnLastInstance : noop, + isInstanceVisibleInViewport: editor + ? editor.isInstanceVisibleInViewport + : () => false, + scrollViewToLastInstance: editor + ? editor.scrollViewToLastInstance + : noop, + scrollViewToPoint: editor ? editor.scrollViewToPoint : noop, getLastCursorSceneCoordinates: editor ? editor.getLastCursorSceneCoordinates : () => [0, 0], @@ -318,6 +325,7 @@ const MosaicEditorsDisplay: React.ComponentType<{ onObjectsModified={props.onObjectsModified} onEffectAdded={props.onEffectAdded} onInstancesModified={_onInstancesModified} + onScenePropertiesModified={props.onScenePropertiesModified} onGetInstanceSize={getInstanceSize} ref={instanceOrObjectPropertiesEditorRef} unsavedChanges={props.unsavedChanges} diff --git a/newIDE/app/src/SceneEditor/SwipeableDrawerEditorsDisplay/index.js b/newIDE/app/src/SceneEditor/SwipeableDrawerEditorsDisplay/index.js index 9b429bf1e74c..2f4a81f801ae 100644 --- a/newIDE/app/src/SceneEditor/SwipeableDrawerEditorsDisplay/index.js +++ b/newIDE/app/src/SceneEditor/SwipeableDrawerEditorsDisplay/index.js @@ -249,6 +249,13 @@ const SwipeableDrawerEditorsDisplay: React.ComponentType<{ centerViewOnLastInstance: editor ? editor.centerViewOnLastInstance : noop, + isInstanceVisibleInViewport: editor + ? editor.isInstanceVisibleInViewport + : () => false, + scrollViewToLastInstance: editor + ? editor.scrollViewToLastInstance + : noop, + scrollViewToPoint: editor ? editor.scrollViewToPoint : noop, getLastCursorSceneCoordinates: editor ? editor.getLastCursorSceneCoordinates : () => [0, 0], @@ -481,6 +488,9 @@ const SwipeableDrawerEditorsDisplay: React.ComponentType<{ onObjectsModified={props.onObjectsModified} onEffectAdded={props.onEffectAdded} onInstancesModified={forceUpdateInstancesList} + onScenePropertiesModified={ + props.onScenePropertiesModified + } onGetInstanceSize={getInstanceSize} ref={instanceOrObjectPropertiesEditorRef} historyHandler={props.historyHandler} diff --git a/newIDE/app/src/SceneEditor/index.js b/newIDE/app/src/SceneEditor/index.js index 63f2d8fb1362..a33beda73cc6 100644 --- a/newIDE/app/src/SceneEditor/index.js +++ b/newIDE/app/src/SceneEditor/index.js @@ -37,13 +37,15 @@ import { type PreviewDebuggerServer } from '../ExportAndShare/PreviewLauncher.fl import EditSceneIcon from '../UI/CustomSvgIcons/EditScene'; import { type HistoryState, - undo, - redo, + type CompositeTargets, canUndo, canRedo, - getHistoryInitialState, - saveToHistory, + getCompositeHistoryInitialState, + saveCompositeToHistory, + undoComposite, + redoComposite, } from '../Utils/History'; +import { diffInstancesSnapshots } from '../Utils/InstancesSnapshotDiff'; import PixiResourcesLoader from '../ObjectsRendering/PixiResourcesLoader'; import { type ObjectWithContext, @@ -95,6 +97,7 @@ import { type EditorViewPosition2D } from '../InstancesEditor'; import { changeViewPosition, setCameraState, + focusEmbeddedGameFrame, } from '../EmbeddedGame/EmbeddedGameFrame'; import Rectangle from '../Utils/Rectangle'; import { exceptionallyGuardAgainstDeadObject } from '../Utils/IsNullPtr'; @@ -349,7 +352,7 @@ export default class SceneEditor extends React.Component { extractAsCustomObjectDialogOpen: false, instancesEditorSettings: initialInstancesEditorSettings, - history: getHistoryInitialState(props.initialInstances, { + history: getCompositeHistoryInitialState(this._getHistoryTargets(), { historyMaxSize: 50, }), @@ -378,6 +381,18 @@ export default class SceneEditor extends React.Component { if (this.state.history !== prevState.history) if (this.props.unsavedChanges) this.props.unsavedChanges.triggerUnsavedChanges(); + + // When the editor tab becomes active again, the focus can be lost + // (staying on the previous tab or on the document body): take it back + // so the keyboard shortcuts work without a click in the editor. Same + // when switching between the 2D and 3D editors, which don't listen to + // the keyboard on the same element. + if ( + (!prevProps.isActive && this.props.isActive) || + prevProps.gameEditorMode !== this.props.gameEditorMode + ) { + this._ensureKeyboardFocusStaysInEditor(); + } } componentDidMount() { @@ -462,7 +477,7 @@ export default class SceneEditor extends React.Component { } // Cancelled, not flushed: the history lives in the state of this // component, so there is nothing left to save it to. - this._saveInstancesModificationsToHistoryDebounced.cancel(); + this._savePendingModificationsToHistoryDebounced.cancel(); } onEditorReloaded() { @@ -583,9 +598,9 @@ export default class SceneEditor extends React.Component { this.setState( { selectedObjectFolderOrObjectsWithContext: [], - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'DELETE' ), }, @@ -800,6 +815,20 @@ export default class SceneEditor extends React.Component { this.forceUpdateObjectGroupsList(); }; + /** + * The toolbar is rendered by the app outside of the editor: a click on + * one of its buttons moves the focus on it, killing the editor keyboard + * shortcuts (like undo/redo). Wrap the actions staying in the editor so + * they take the focus back (actions opening a dialog are left untouched: + * the dialog takes the focus). + */ + _withFocusReturnedToEditor = ( + action: (...args: Array) => void + ): ((...args: Array) => void) => (...args) => { + action(...args); + this._ensureKeyboardFocusStaysInEditor(); + }; + updateToolbar = () => { const { editorDisplay } = this; if (!editorDisplay) return; @@ -808,29 +837,45 @@ export default class SceneEditor extends React.Component { this.props.setToolbar( { this.props.setToolbar( { ): any); /** - * Return the history, with any instance modification pending a debounced - * save (see `_onInstancesModified`) committed to it. Must be used by - * undo/redo: an uncommitted modification would otherwise be reverted - * without being redoable (`undo` restores the last *saved* snapshot). + * The state tracked by the undo/redo history: everything owned by the + * edited layout that the editor can modify - the instances, the layers + * (a layer deletion also deletes its instances: tracking both makes it a + * single, consistent undoable step) and the scene properties. + * Not tracked (not owned by the layout): objects, object groups, events. + */ + _getHistoryTargets = (): CompositeTargets => { + const { layout } = this.props; + return { + instances: { + serializableObject: this.props.initialInstances, + unserializationNeedsProject: true, + }, + layers: { + serializableObject: this.props.layersContainer, + serializationMethodName: 'serializeLayersTo', + unserializationMethodName: 'unserializeLayersFrom', + }, + // Scene variables are intentionally NOT tracked here: the variables + // list keeps its own local history. State that would be captured in + // the snapshots without being saved on each change would be silently + // reverted by an undo (and not re-appliable by a redo). + ...(layout + ? { + sceneProperties: { + getValue: () => ({ + windowDefaultTitle: layout.getWindowDefaultTitle(), + stopSoundsOnStartup: layout.stopSoundsOnStartup(), + resourcesPreloading: layout.getResourcesPreloading(), + resourcesUnloading: layout.getResourcesUnloading(), + backgroundColorRed: layout.getBackgroundColorRed(), + backgroundColorGreen: layout.getBackgroundColorGreen(), + backgroundColorBlue: layout.getBackgroundColorBlue(), + }), + setValue: (value: Object) => { + layout.setWindowDefaultTitle(value.windowDefaultTitle); + layout.setStopSoundsOnStartup(value.stopSoundsOnStartup); + layout.setResourcesPreloading(value.resourcesPreloading); + layout.setResourcesUnloading(value.resourcesUnloading); + layout.setBackgroundColor( + value.backgroundColorRed, + value.backgroundColorGreen, + value.backgroundColorBlue + ); + }, + }, + } + : {}), + }; + }; + + /** + * Return the history, with any modification pending a debounced save + * (see `_onInstancesModified`, `_onScenePropertiesModified`) committed to + * it. Must be used by undo/redo: an uncommitted modification would + * otherwise be reverted without being redoable (`undo` restores the last + * *saved* snapshot). */ - _getHistoryWithPendingInstancesModificationsSaved = (): HistoryState => { - this._saveInstancesModificationsToHistoryDebounced.cancel(); - if (!this._hasPendingInstancesModificationsToSave) { + _getHistoryWithPendingModificationsSaved = (): HistoryState => { + this._savePendingModificationsToHistoryDebounced.cancel(); + if (!this._hasPendingModificationsToSave) { return this.state.history; } - this._hasPendingInstancesModificationsToSave = false; - return saveToHistory(this.state.history, this.props.initialInstances); + this._hasPendingModificationsToSave = false; + return saveCompositeToHistory( + this.state.history, + this._getHistoryTargets() + ); }; undo = () => { - // /!\ Drop the selection to avoid keeping any references to deleted instances. - // This could be avoided if the selection used something like UUID to address instances. + const history = this._getHistoryWithPendingModificationsSaved(); + if (!canUndo(history)) { + // Nothing to undo - but a pending modification may just have been + // committed to the history: keep it. + if (history !== this.state.history) this.setState({ history }); + return; + } + + const selectedInstancesPersistentUuids = this.instancesSelection + .getSelectedInstances() + .map(instance => instance.getPersistentUuid()); + + // /!\ Drop the selection to avoid keeping any references to instances + // that are deleted (or re-created) when the history is applied. this.instancesSelection.clearSelection(); + const valueBeforeChange = history.currentValue; + const newHistory = undoComposite( + history, + this._getHistoryTargets(), + this.props.project + ); this.setState( { - history: undo( - this._getHistoryWithPendingInstancesModificationsSaved(), - this.props.initialInstances, - this.props.project - ), + history: newHistory, }, () => { // /!\ Force the instances editor to destroy and mount again the // renderers to avoid keeping any references to existing instances if (this.editorDisplay) this.editorDisplay.instancesHandlers.forceRemountInstancesRenderers(); + + // Select the instances touched by the change, so it can be seen - + // notably in the 3D editor, where the camera is not moved (like + // other engines do: the change is revealed by the selection). If no + // instance was touched, restore the previous selection. + const { changedOrAddedPersistentUuids } = diffInstancesSnapshots( + valueBeforeChange.instances, + newHistory.currentValue.instances + ); + const persistentUuidsToSelect = + changedOrAddedPersistentUuids.length > 0 + ? changedOrAddedPersistentUuids + : selectedInstancesPersistentUuids; + persistentUuidsToSelect.forEach(persistentUuid => { + const instance = getInstanceInLayoutWithPersistentUuid( + this.props.initialInstances, + persistentUuid + ); + if (instance) + this.instancesSelection.selectInstance({ + instance, + multiSelect: true, + layersLocks: null, + }); + }); + this.forceUpdatePropertiesEditor(); + this._sendSelectedInstances(); + this._ensureKeyboardFocusStaysInEditor(); + + this.forceUpdateLayersList(); this.updateToolbar(); this._sendHotReloadAllInstances(); + this._sendHotReloadLayers(); + + this._revealHistoryChanges(valueBeforeChange, newHistory.currentValue); } ); }; redo = () => { - // /!\ Drop the selection to avoid keeping any references to deleted instances. - // This could be avoided if the selection used something like UUID to address instances. + const history = this._getHistoryWithPendingModificationsSaved(); + if (!canRedo(history)) { + // Nothing to redo - but a pending modification may just have been + // committed to the history: keep it. + if (history !== this.state.history) this.setState({ history }); + return; + } + + const selectedInstancesPersistentUuids = this.instancesSelection + .getSelectedInstances() + .map(instance => instance.getPersistentUuid()); + + // /!\ Drop the selection to avoid keeping any references to instances + // that are deleted (or re-created) when the history is applied. this.instancesSelection.clearSelection(); + const valueBeforeChange = history.currentValue; + const newHistory = redoComposite( + history, + this._getHistoryTargets(), + this.props.project + ); this.setState( { - history: redo( - this._getHistoryWithPendingInstancesModificationsSaved(), - this.props.initialInstances, - this.props.project - ), + history: newHistory, }, () => { // /!\ Force the instances editor to destroy and mount again the // renderers to avoid keeping any references to existing instances if (this.editorDisplay) this.editorDisplay.instancesHandlers.forceRemountInstancesRenderers(); + + // Select the instances touched by the change, so it can be seen - + // notably in the 3D editor, where the camera is not moved (like + // other engines do: the change is revealed by the selection). If no + // instance was touched, restore the previous selection. + const { changedOrAddedPersistentUuids } = diffInstancesSnapshots( + valueBeforeChange.instances, + newHistory.currentValue.instances + ); + const persistentUuidsToSelect = + changedOrAddedPersistentUuids.length > 0 + ? changedOrAddedPersistentUuids + : selectedInstancesPersistentUuids; + persistentUuidsToSelect.forEach(persistentUuid => { + const instance = getInstanceInLayoutWithPersistentUuid( + this.props.initialInstances, + persistentUuid + ); + if (instance) + this.instancesSelection.selectInstance({ + instance, + multiSelect: true, + layersLocks: null, + }); + }); + this.forceUpdatePropertiesEditor(); + this._sendSelectedInstances(); + this._ensureKeyboardFocusStaysInEditor(); + + this.forceUpdateLayersList(); this.updateToolbar(); this._sendHotReloadAllInstances(); + this._sendHotReloadLayers(); + + this._revealHistoryChanges(valueBeforeChange, newHistory.currentValue); } ); }; @@ -1360,9 +1571,9 @@ export default class SceneEditor extends React.Component { this.setState( { - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'ADD' ), }, @@ -1474,9 +1685,9 @@ export default class SceneEditor extends React.Component { _onInstancesMoved = (instances: Array) => { this.setState( { - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'EDIT' ), }, @@ -1488,9 +1699,9 @@ export default class SceneEditor extends React.Component { _onInstancesResized = (instances: Array) => { this.setState( { - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'EDIT' ), }, @@ -1502,9 +1713,9 @@ export default class SceneEditor extends React.Component { _onInstancesRotated = (instances: Array) => { this.setState( { - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'EDIT' ), }, @@ -1524,18 +1735,29 @@ export default class SceneEditor extends React.Component { // Save the modification for undo/redo, debounced so a rapid series of // changes (like typing a position digit by digit) makes a single // undoable step. - this._hasPendingInstancesModificationsToSave = true; - this._saveInstancesModificationsToHistoryDebounced(); + this._hasPendingModificationsToSave = true; + this._savePendingModificationsToHistoryDebounced(); + }; + + _onScenePropertiesModified = () => { + // Save the modification for undo/redo, debounced so a rapid series of + // changes (like dragging in the background color picker) makes a + // single undoable step. + this._hasPendingModificationsToSave = true; + this._savePendingModificationsToHistoryDebounced(); }; - _hasPendingInstancesModificationsToSave = false; + _hasPendingModificationsToSave = false; // $FlowFixMe[missing-local-annot] - _saveInstancesModificationsToHistoryDebounced = (debounce(() => { - if (!this._hasPendingInstancesModificationsToSave) return; - this._hasPendingInstancesModificationsToSave = false; + _savePendingModificationsToHistoryDebounced = (debounce(() => { + if (!this._hasPendingModificationsToSave) return; + this._hasPendingModificationsToSave = false; this.setState({ - history: saveToHistory(this.state.history, this.props.initialInstances), + history: saveCompositeToHistory( + this.state.history, + this._getHistoryTargets() + ), }); }, 500): any); @@ -1790,6 +2012,16 @@ export default class SceneEditor extends React.Component { } } + if (doRemove) { + this.setState({ + history: saveCompositeToHistory( + this.state.history, + this._getHistoryTargets(), + 'DELETE' + ), + }); + } + done(doRemove); // /!\ Force the instances editor to destroy and mount again the // renderers to avoid keeping any references to existing instances @@ -1807,6 +2039,12 @@ export default class SceneEditor extends React.Component { }; _onLayerRenamed = () => { + this.setState({ + history: saveCompositeToHistory( + this.state.history, + this._getHistoryTargets() + ), + }); this.forceUpdatePropertiesEditor(); }; @@ -1857,6 +2095,13 @@ export default class SceneEditor extends React.Component { }; _onLayersModified = (hasAnyEffectBeenAdded: boolean) => { + this.setState({ + history: saveCompositeToHistory( + this.state.history, + this._getHistoryTargets() + ), + }); + const { onEffectAdded } = this.props; if (hasAnyEffectBeenAdded) { // This triggers a full hot-reload. We don't need to reload layers specifically. @@ -2277,9 +2522,9 @@ export default class SceneEditor extends React.Component { this.setState( { selectedObjectFolderOrObjectsWithContext: [], - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances, + this._getHistoryTargets(), 'DELETE' ), }, @@ -3005,6 +3250,147 @@ export default class SceneEditor extends React.Component { }); }; + _containerElement: ?HTMLDivElement = null; + + /** + * If the focused element disappeared (like a button of the properties + * panel unmounted after an undo), the focus falls back on the document + * body - outside of the editor, killing its keyboard shortcuts. Take the + * focus back in that case. + */ + _ensureKeyboardFocusStaysInEditor = () => { + // In the 3D editor, the keyboard is handled by the embedded game: its + // in-game editor has its own shortcuts (like "F" to focus the + // selection) and forwards undo/redo & others back to the editor. Give + // it the focus instead of the editor container. + if ( + this.props.gameEditorMode === 'embedded-game' && + focusEmbeddedGameFrame() + ) { + return; + } + + const containerElement = this._containerElement; + if (!containerElement) return; + const { activeElement } = document; + if ( + !activeElement || + activeElement === document.body || + !containerElement.contains(activeElement) + ) { + containerElement.focus(); + } + }; + + /** + * After an undo/redo, make sure its effect can be seen - otherwise the + * undo/redo feels like it did nothing: + * - a change on state only shown in a panel (layers, scene properties) + * opens this panel, + * - a change on instances all outside of the view moves the view to them. + */ + _revealHistoryChanges = ( + valueBeforeChange: ?Object, + valueAfterChange: ?Object + ) => { + const { editorDisplay } = this; + if (!editorDisplay) return; + const beforeChange: Object = valueBeforeChange || {}; + const afterChange: Object = valueAfterChange || {}; + + const hasChanged = (key: string) => + JSON.stringify(beforeChange[key]) !== JSON.stringify(afterChange[key]); + if (hasChanged('layers')) { + editorDisplay.ensureEditorVisible('layers-list'); + } + if (hasChanged('sceneProperties')) { + // Scene properties are only shown in the properties panel, and only + // when nothing is selected: deselect and open the panel so the + // undone/redone change can be seen. + this.instancesSelection.clearSelection(); + this.setState({ lastSelectionType: 'instance' }); + this.forceUpdatePropertiesEditor(); + this._sendSelectedInstances(); + editorDisplay.ensureEditorVisible('properties'); + } + + const { + changedOrAddedPersistentUuids, + removedInstances, + } = diffInstancesSnapshots(beforeChange.instances, afterChange.instances); + + const changedInstances = changedOrAddedPersistentUuids + .map(persistentUuid => + getInstanceInLayoutWithPersistentUuid( + this.props.initialInstances, + persistentUuid + ) + ) + .filter(Boolean); + + const removedPositions: Array<[number, number]> = removedInstances.map( + instance => [Number(instance.x) || 0, Number(instance.y) || 0] + ); + if (changedInstances.length === 0 && removedPositions.length === 0) return; + + const viewPosition = editorDisplay.viewControls.getViewPosition(); + if (!viewPosition) return; + // If at least one touched instance is visible - even partly - the user + // can already see the effect of the undo/redo: don't move the view. + if ( + changedInstances.some(instance => + editorDisplay.viewControls.isInstanceVisibleInViewport(instance) + ) + ) + return; + if (removedPositions.some(([x, y]) => viewPosition.containsPoint(x, y))) + return; + + if (changedInstances.length > 0) { + editorDisplay.viewControls.scrollViewToLastInstance(changedInstances); + } else { + // Only deletions: bring the view to where the last deleted + // instance was. + const [x, y] = removedPositions[removedPositions.length - 1]; + editorDisplay.viewControls.scrollViewToPoint(x, y); + } + }; + + _onKeyDownInEditor = (evt: SyntheticKeyboardEvent) => { + // The instances editor canvas handles its own shortcuts (including + // undo/redo): don't handle the same event twice. + if (evt.nativeEvent.defaultPrevented) return; + // React events bubble through the React tree, not the DOM tree: a + // shortcut pressed inside a dialog (rendered in a portal, on top of the + // editor) reaches this handler too. Ignore it - the dialog handles (or + // ignores) its own shortcuts. + if ( + evt.target instanceof Node && + this._containerElement && + !this._containerElement.contains(evt.target) + ) { + return; + } + // Let text fields handle their own undo/redo. + if ( + evt.target instanceof Element && + evt.target.closest('textarea, input, [contenteditable="true"]') + ) { + return; + } + if (!evt.ctrlKey && !evt.metaKey) return; + + const key = evt.key.toLowerCase(); + if (key === 'z') { + evt.preventDefault(); + if (evt.shiftKey) this.redo(); + else this.undo(); + } else if (key === 'y') { + evt.preventDefault(); + this.redo(); + } + }; + render(): any { const { project, @@ -3072,6 +3458,14 @@ export default class SceneEditor extends React.Component { style={styles.container} id="scene-editor" data-active={isActive ? 'true' : undefined} + onKeyDown={this._onKeyDownInEditor} + // Allow the container to receive the focus (see + // `_ensureKeyboardFocusStaysInEditor`), so the keyboard + // shortcuts keep working after an undo/redo. + tabIndex={-1} + ref={containerElement => + (this._containerElement = containerElement) + } > { instancesSelection={this.instancesSelection} onSelectInstances={this._onSelectInstances} onInstancesModified={this._onInstancesModified} + onScenePropertiesModified={this._onScenePropertiesModified} onAddObjectInstance={this.addInstanceOnTheScene} chosenLayer={this.state.chosenLayer} onChooseLayer={this._onChooseLayer} @@ -3175,9 +3570,9 @@ export default class SceneEditor extends React.Component { canRedo: () => canRedo(this.state.history), saveToHistory: () => this.setState({ - history: saveToHistory( + history: saveCompositeToHistory( this.state.history, - this.props.initialInstances + this._getHistoryTargets() ), }), }} @@ -3512,7 +3907,15 @@ export default class SceneEditor extends React.Component { project={project} layout={layout} onClose={() => this.openSceneProperties(false)} - onApply={() => this.openSceneProperties(false)} + onApply={() => { + this.setState({ + history: saveCompositeToHistory( + this.state.history, + this._getHistoryTargets() + ), + }); + this.openSceneProperties(false); + }} onEditVariables={() => this.openSceneVariables(true)} onOpenMoreSettings={this.props.onOpenMoreSettings} resourceManagementProps={ diff --git a/newIDE/app/src/Utils/CompositeHistory.spec.js b/newIDE/app/src/Utils/CompositeHistory.spec.js new file mode 100644 index 000000000000..e04dcad6097f --- /dev/null +++ b/newIDE/app/src/Utils/CompositeHistory.spec.js @@ -0,0 +1,129 @@ +// @flow +import { + getCompositeHistoryInitialState, + saveCompositeToHistory, + undoComposite, + redoComposite, + canUndo, + canRedo, + type CompositeTargets, +} from './History'; + +const gd: libGDevelop = global.gd; + +describe('composite history', () => { + let variablesContainer: gdVariablesContainer; + let customState: { color: string }; + let targets: CompositeTargets; + + beforeEach(() => { + variablesContainer = new gd.VariablesContainer( + gd.VariablesContainer.Unknown + ); + customState = { color: 'red' }; + targets = { + variables: { serializableObject: variablesContainer }, + custom: { + getValue: () => ({ ...customState }), + setValue: value => { + customState = { ...value }; + }, + }, + }; + }); + + afterEach(() => { + variablesContainer.delete(); + }); + + it('captures the changes of all the targets in a single undoable step', () => { + let history = getCompositeHistoryInitialState(targets, { + historyMaxSize: 10, + }); + expect(canUndo(history)).toBe(false); + + // Change both targets, then save once. + variablesContainer.insertNew('Score', 0).setValue(100); + customState.color = 'blue'; + history = saveCompositeToHistory(history, targets); + expect(canUndo(history)).toBe(true); + + // Undo restores both targets. + history = undoComposite(history, targets); + expect(variablesContainer.has('Score')).toBe(false); + expect(customState.color).toBe('red'); + expect(canUndo(history)).toBe(false); + expect(canRedo(history)).toBe(true); + + // Redo re-applies both. + history = redoComposite(history, targets); + expect(variablesContainer.has('Score')).toBe(true); + expect(variablesContainer.get('Score').getValue()).toBe(100); + expect(customState.color).toBe('blue'); + expect(canRedo(history)).toBe(false); + }); + + it('supports custom serialization methods and per-target project passing', () => { + const project = gd.ProjectHelper.createNewGDJSProject(); + const layout = project.insertNewLayout('Scene', 0); + layout.getObjects().insertNewObject(project, 'Sprite', 'Player', 0); + + // The same targets as the scene editor: the instances unserialization + // requires the project as first argument, the layers one breaks if it + // is given. + const sceneTargets: CompositeTargets = { + instances: { + serializableObject: layout.getInitialInstances(), + unserializationNeedsProject: true, + }, + layers: { + serializableObject: layout.getLayers(), + serializationMethodName: 'serializeLayersTo', + unserializationMethodName: 'unserializeLayersFrom', + }, + }; + + let history = getCompositeHistoryInitialState(sceneTargets, { + historyMaxSize: 10, + }); + const instance = layout.getInitialInstances().insertNewInitialInstance(); + instance.setObjectName('Player'); + instance.setHidden(true); + layout.getLayers().insertNewLayer('UI', 1); + layout + .getLayers() + .getLayer('UI') + .setVisibility(false); + history = saveCompositeToHistory(history, sceneTargets); + + history = undoComposite(history, sceneTargets, project); + expect(layout.getLayers().hasLayerNamed('UI')).toBe(false); + expect(layout.getInitialInstances().getInstancesCount()).toBe(0); + + redoComposite(history, sceneTargets, project); + expect(layout.getLayers().hasLayerNamed('UI')).toBe(true); + expect( + layout + .getLayers() + .getLayer('UI') + .getVisibility() + ).toBe(false); + expect(layout.getInitialInstances().getInstancesCount()).toBe(1); + + project.delete(); + }); + + it('empties the redo stack on a new save', () => { + let history = getCompositeHistoryInitialState(targets, { + historyMaxSize: 10, + }); + customState.color = 'blue'; + history = saveCompositeToHistory(history, targets); + history = undoComposite(history, targets); + expect(canRedo(history)).toBe(true); + + customState.color = 'green'; + history = saveCompositeToHistory(history, targets); + expect(canRedo(history)).toBe(false); + }); +}); diff --git a/newIDE/app/src/Utils/History.js b/newIDE/app/src/Utils/History.js index 51774ba9cb2b..e2bb98388f27 100644 --- a/newIDE/app/src/Utils/History.js +++ b/newIDE/app/src/Utils/History.js @@ -183,3 +183,178 @@ export const redo = ( currentValue: newCurrentValue, }; }; + +// Composite history: track several pieces of state as a single history +// (so one undoable step can capture changes across all of them - for +// example the deletion of a layer together with the instances it contained). + +export type CompositeTarget = { + serializableObject?: gdSerializable, + serializationMethodName?: string, + unserializationMethodName?: string, + // The unserialization method of some gd.* classes requires the project as + // first argument - and others break if it is given. + unserializationNeedsProject?: boolean, + // For state without its own serializer (like a few scene properties): + getValue?: () => Object, + setValue?: (value: Object) => void, +}; + +export type CompositeTargets = { [key: string]: CompositeTarget }; + +const serializeCompositeTargets = (targets: CompositeTargets): Object => { + const value: { [key: string]: Object } = {}; + for (const key of Object.keys(targets)) { + const target = targets[key]; + if (target.serializableObject) { + value[key] = serializeToJSObject( + target.serializableObject, + target.serializationMethodName || 'serializeTo' + ); + } else if (target.getValue) { + value[key] = target.getValue(); + } + } + return value; +}; + +const unserializeCompositeTargets = ( + targets: CompositeTargets, + value: Object, + project: ?gdProject +): void => { + for (const key of Object.keys(targets)) { + const target = targets[key]; + if (value[key] === undefined) continue; + + if (target.serializableObject) { + unserializeFromJSObject( + target.serializableObject, + value[key], + target.unserializationMethodName || 'unserializeFrom', + target.unserializationNeedsProject ? project : undefined + ); + } else if (target.setValue) { + target.setValue(value[key]); + } + } +}; + +/** + * Return the initial state of a history tracking several pieces of state. + */ +export const getCompositeHistoryInitialState = ( + targets: CompositeTargets, + { + historyMaxSize, + }: { + historyMaxSize: number, + } +): HistoryState => { + return { + previousActions: [], + currentValue: serializeCompositeTargets(targets), + futureActions: [], + maxSize: historyMaxSize, + }; +}; + +/** + * Save a new state of the given targets to the history. + */ +export const saveCompositeToHistory = ( + history: HistoryState, + targets: CompositeTargets, + actionType?: RevertableActionType, + changeContext?: any +): HistoryState => { + const newCurrentValue = serializeCompositeTargets(targets); + const newPreviousActions = [ + ...history.previousActions, + { + type: actionType, + valueBeforeChange: history.currentValue, + changeContext, + }, + ]; + const newFutureActions: Array = []; // Empty the future actions on save. + if (newPreviousActions.length > history.maxSize) { + newPreviousActions.splice(0, newPreviousActions.length - history.maxSize); + } + + return { + ...history, + currentValue: newCurrentValue, + previousActions: newPreviousActions, + futureActions: newFutureActions, + }; +}; + +/** + * Update the targets to undo the last changes. + * /!\ This mutates the objects of the targets and there could be objects + * owned by them deleted or becoming invalid. Be sure to drop/refresh any + * reference to them. + */ +export const undoComposite = ( + history: HistoryState, + targets: CompositeTargets, + project: ?gdProject = undefined +): HistoryState => { + if (!history.previousActions.length) { + return history; + } + + const previousAction = + history.previousActions[history.previousActions.length - 1]; + const newCurrentValue = previousAction.valueBeforeChange; + unserializeCompositeTargets(targets, newCurrentValue, project); + + return { + ...history, + previousActions: history.previousActions.slice(0, -1), + futureActions: [ + ...history.futureActions, + { + type: previousAction.type, + changeContext: previousAction.changeContext, + valueAfterChange: history.currentValue, + }, + ], + currentValue: newCurrentValue, + }; +}; + +/** + * Update the targets to redo the last undone changes. + * /!\ This mutates the objects of the targets and there could be objects + * owned by them deleted or becoming invalid. Be sure to drop/refresh any + * reference to them. + */ +export const redoComposite = ( + history: HistoryState, + targets: CompositeTargets, + project: ?gdProject = undefined +): HistoryState => { + if (!history.futureActions.length) { + return history; + } + + const futureAction = history.futureActions[history.futureActions.length - 1]; + const newCurrentValue = futureAction.valueAfterChange; + unserializeCompositeTargets(targets, newCurrentValue, project); + + return { + ...history, + previousActions: [ + ...history.previousActions, + { + type: futureAction.type, + changeContext: futureAction.changeContext, + valueBeforeChange: history.currentValue, + }, + ], + futureActions: history.futureActions.slice(0, -1), + currentValue: newCurrentValue, + }; +}; diff --git a/newIDE/app/src/Utils/InstancesSnapshotDiff.js b/newIDE/app/src/Utils/InstancesSnapshotDiff.js new file mode 100644 index 000000000000..173782b41ec6 --- /dev/null +++ b/newIDE/app/src/Utils/InstancesSnapshotDiff.js @@ -0,0 +1,53 @@ +// @flow + +// The scene editor history stores the initial instances as a serialized +// snapshot: an array of plain objects, each with a `persistentUuid` +// (guaranteed: instances get one at creation and it survives serialization). +// Diffing two snapshots tells which instances an undo/redo touched - used +// to reveal the change in the editor when it happened outside of the view. + +export type InstancesSnapshotDiff = {| + // Instances added or modified by the change (they exist after it). + changedOrAddedPersistentUuids: Array, + // Snapshots of the instances removed by the change (with their last + // known position). + removedInstances: Array, +|}; + +export const diffInstancesSnapshots = ( + instancesBeforeChange: ?Array, + instancesAfterChange: ?Array +): InstancesSnapshotDiff => { + const instancesByUuidBeforeChange = new Map(); + (instancesBeforeChange || []).forEach(instance => { + if (instance.persistentUuid) + instancesByUuidBeforeChange.set(instance.persistentUuid, instance); + }); + + const changedOrAddedPersistentUuids = []; + const uuidsAfterChange = new Set(); + (instancesAfterChange || []).forEach(instance => { + const persistentUuid = instance.persistentUuid; + if (!persistentUuid) return; + uuidsAfterChange.add(persistentUuid); + + const instanceBeforeChange = instancesByUuidBeforeChange.get( + persistentUuid + ); + // Both snapshots come from the same serializer, so a same instance + // always serializes the same way: a JSON comparison is enough. + if ( + !instanceBeforeChange || + JSON.stringify(instanceBeforeChange) !== JSON.stringify(instance) + ) { + changedOrAddedPersistentUuids.push(persistentUuid); + } + }); + + const removedInstances = []; + instancesByUuidBeforeChange.forEach((instance, persistentUuid) => { + if (!uuidsAfterChange.has(persistentUuid)) removedInstances.push(instance); + }); + + return { changedOrAddedPersistentUuids, removedInstances }; +}; diff --git a/newIDE/app/src/Utils/InstancesSnapshotDiff.spec.js b/newIDE/app/src/Utils/InstancesSnapshotDiff.spec.js new file mode 100644 index 000000000000..fe375c9aacf5 --- /dev/null +++ b/newIDE/app/src/Utils/InstancesSnapshotDiff.spec.js @@ -0,0 +1,78 @@ +// @flow +import { diffInstancesSnapshots } from './InstancesSnapshotDiff'; +import { serializeToJSObject } from './Serializer'; + +const gd: libGDevelop = global.gd; + +describe('diffInstancesSnapshots', () => { + it('finds nothing on identical snapshots', () => { + const snapshot = [{ persistentUuid: 'aaa', name: 'Player', x: 0, y: 0 }]; + const diff = diffInstancesSnapshots(snapshot, snapshot); + expect(diff.changedOrAddedPersistentUuids).toEqual([]); + expect(diff.removedInstances).toEqual([]); + }); + + it('finds added, modified and removed instances', () => { + const before = [ + { persistentUuid: 'aaa', name: 'Player', x: 0, y: 0 }, + { persistentUuid: 'bbb', name: 'Enemy', x: 10, y: 10 }, + ]; + const after = [ + { persistentUuid: 'aaa', name: 'Player', x: 500, y: 0 }, // Moved. + { persistentUuid: 'ccc', name: 'Coin', x: 20, y: 20 }, // Added. + // 'bbb' removed. + ]; + + const diff = diffInstancesSnapshots(before, after); + expect(diff.changedOrAddedPersistentUuids).toEqual(['aaa', 'ccc']); + expect(diff.removedInstances).toEqual([ + { persistentUuid: 'bbb', name: 'Enemy', x: 10, y: 10 }, + ]); + }); + + it('handles missing snapshots', () => { + const snapshot = [{ persistentUuid: 'aaa', name: 'Player', x: 0, y: 0 }]; + expect(diffInstancesSnapshots(null, snapshot)).toEqual({ + changedOrAddedPersistentUuids: ['aaa'], + removedInstances: [], + }); + expect(diffInstancesSnapshots(snapshot, null)).toEqual({ + changedOrAddedPersistentUuids: [], + removedInstances: [snapshot[0]], + }); + }); + + // The diff is used on snapshots of `gd.InitialInstancesContainer` stored in + // the scene editor history: lock their shape (an array of objects with a + // `persistentUuid`) with the real serializer. + it('works on real serialized initial instances', () => { + const project = gd.ProjectHelper.createNewGDJSProject(); + const layout = project.insertNewLayout('Scene', 0); + const initialInstances = layout.getInitialInstances(); + const playerInstance = initialInstances.insertNewInitialInstance(); + playerInstance.setObjectName('Player'); + const enemyInstance = initialInstances.insertNewInitialInstance(); + enemyInstance.setObjectName('Enemy'); + + const snapshotBeforeChange = serializeToJSObject(initialInstances); + expect(Array.isArray(snapshotBeforeChange)).toBe(true); + expect(snapshotBeforeChange.length).toBe(2); + snapshotBeforeChange.forEach(instance => + expect(instance.persistentUuid).toBeTruthy() + ); + + playerInstance.setX(1000); + const snapshotAfterChange = serializeToJSObject(initialInstances); + + const diff = diffInstancesSnapshots( + snapshotBeforeChange, + snapshotAfterChange + ); + expect(diff.changedOrAddedPersistentUuids).toEqual([ + playerInstance.getPersistentUuid(), + ]); + expect(diff.removedInstances).toEqual([]); + + project.delete(); + }); +}); From cd7c7e7ae85e2c6de81cc73e827bb1cb02f19384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Fri, 31 Jul 2026 17:07:31 +0200 Subject: [PATCH 3/3] wip highlight row --- newIDE/app/src/SceneEditor/UndoRedoFlash.css | 15 ++ newIDE/app/src/SceneEditor/index.js | 171 +++++++++++++++++-- 2 files changed, 170 insertions(+), 16 deletions(-) create mode 100644 newIDE/app/src/SceneEditor/UndoRedoFlash.css diff --git a/newIDE/app/src/SceneEditor/UndoRedoFlash.css b/newIDE/app/src/SceneEditor/UndoRedoFlash.css new file mode 100644 index 000000000000..4ac88549a2f2 --- /dev/null +++ b/newIDE/app/src/SceneEditor/UndoRedoFlash.css @@ -0,0 +1,15 @@ +/* Brief highlight of a properties panel row changed by an undo/redo + (see `_flashChangedInstancePropertyRows` in `SceneEditor/index.js`). */ +@keyframes undo-redo-property-flash { + 0% { + background-color: rgba(255, 176, 32, 0.5); + } + 100% { + background-color: transparent; + } +} + +.undo-redo-property-flash { + animation: undo-redo-property-flash 1.2s ease-out; + border-radius: 4px; +} diff --git a/newIDE/app/src/SceneEditor/index.js b/newIDE/app/src/SceneEditor/index.js index a33beda73cc6..9b67f7c62040 100644 --- a/newIDE/app/src/SceneEditor/index.js +++ b/newIDE/app/src/SceneEditor/index.js @@ -46,6 +46,8 @@ import { redoComposite, } from '../Utils/History'; import { diffInstancesSnapshots } from '../Utils/InstancesSnapshotDiff'; +import './UndoRedoFlash.css'; + import PixiResourcesLoader from '../ObjectsRendering/PixiResourcesLoader'; import { type ObjectWithContext, @@ -112,6 +114,28 @@ import { type ObjectGroupEditorTab } from '../ObjectGroupEditor/EditedObjectGrou const gd: libGDevelop = global.gd; +// How the attributes of a serialized instance (as found in the history +// snapshots) map to the field ids of the compact instance properties editor +// (see `CompactInstancePropertiesSchema.js`). +const serializedInstanceKeyToPropertyFieldId: { [string]: string } = { + x: 'X', + y: 'Y', + z: 'Z', + angle: 'Angle', + rotationX: 'Rotation X', + rotationY: 'Rotation Y', + zOrder: 'Z Order', + layer: 'Layer', + width: 'Width', + height: 'Height', + depth: 'Depth', + // Toggling the custom size is seen in the size fields. + customSize: 'Width', + customDepth: 'Depth', + hidden: 'Hide instance', + locked: 'Lock instance', +}; + const BASE_LAYER_NAME = ''; const INSTANCES_CLIPBOARD_KIND = 'Instances'; @@ -601,7 +625,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'DELETE' + 'DELETE', + { source: 'canvas' } ), }, () => { @@ -1296,7 +1321,9 @@ export default class SceneEditor extends React.Component { this._hasPendingModificationsToSave = false; return saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ); }; @@ -1317,6 +1344,9 @@ export default class SceneEditor extends React.Component { // that are deleted (or re-created) when the history is applied. this.instancesSelection.clearSelection(); const valueBeforeChange = history.currentValue; + const undoneActionChangeContext = + history.previousActions[history.previousActions.length - 1] + .changeContext || null; const newHistory = undoComposite( history, this._getHistoryTargets(), @@ -1365,7 +1395,11 @@ export default class SceneEditor extends React.Component { this._sendHotReloadAllInstances(); this._sendHotReloadLayers(); - this._revealHistoryChanges(valueBeforeChange, newHistory.currentValue); + this._revealHistoryChanges( + valueBeforeChange, + newHistory.currentValue, + undoneActionChangeContext + ); } ); }; @@ -1387,6 +1421,9 @@ export default class SceneEditor extends React.Component { // that are deleted (or re-created) when the history is applied. this.instancesSelection.clearSelection(); const valueBeforeChange = history.currentValue; + const undoneActionChangeContext = + history.futureActions[history.futureActions.length - 1].changeContext || + null; const newHistory = redoComposite( history, this._getHistoryTargets(), @@ -1435,7 +1472,11 @@ export default class SceneEditor extends React.Component { this._sendHotReloadAllInstances(); this._sendHotReloadLayers(); - this._revealHistoryChanges(valueBeforeChange, newHistory.currentValue); + this._revealHistoryChanges( + valueBeforeChange, + newHistory.currentValue, + undoneActionChangeContext + ); } ); }; @@ -1574,7 +1615,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'ADD' + 'ADD', + { source: 'canvas' } ), }, () => this.updateToolbar() @@ -1688,7 +1730,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'EDIT' + 'EDIT', + { source: 'canvas' } ), }, () => this.forceUpdatePropertiesEditor() @@ -1702,7 +1745,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'EDIT' + 'EDIT', + { source: 'canvas' } ), }, () => this.forceUpdatePropertiesEditor() @@ -1716,7 +1760,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'EDIT' + 'EDIT', + { source: 'canvas' } ), }, () => this.forceUpdatePropertiesEditor() @@ -1756,7 +1801,9 @@ export default class SceneEditor extends React.Component { this.setState({ history: saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ), }); }, 500): any); @@ -2017,7 +2064,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'DELETE' + 'DELETE', + { source: 'panel' } ), }); } @@ -2042,7 +2090,9 @@ export default class SceneEditor extends React.Component { this.setState({ history: saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ), }); this.forceUpdatePropertiesEditor(); @@ -2098,7 +2148,9 @@ export default class SceneEditor extends React.Component { this.setState({ history: saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ), }); @@ -2525,7 +2577,8 @@ export default class SceneEditor extends React.Component { history: saveCompositeToHistory( this.state.history, this._getHistoryTargets(), - 'DELETE' + 'DELETE', + { source: 'canvas' } ), }, () => { @@ -3282,6 +3335,80 @@ export default class SceneEditor extends React.Component { } }; + /** + * After an undo/redo touching instances, briefly highlight the rows of + * the properties panel showing the values it changed. + */ + _flashChangedInstancePropertyRows = ( + instancesBeforeChange: ?Array, + instancesAfterChange: ?Array, + changedOrAddedPersistentUuids: Array, + changeContext: ?{ source: 'canvas' | 'panel' } + ) => { + if (changedOrAddedPersistentUuids.length === 0) return; + const containerElement = this._containerElement; + if (!containerElement) return; + + const byUuid = (instances: ?Array) => + new Map( + (instances || []) + .filter(instance => instance.persistentUuid) + .map(instance => [instance.persistentUuid, instance]) + ); + const instancesByUuidBeforeChange = byUuid(instancesBeforeChange); + const instancesByUuidAfterChange = byUuid(instancesAfterChange); + + const changedFieldIds = new Set(); + changedOrAddedPersistentUuids.forEach(persistentUuid => { + const beforeInstance = instancesByUuidBeforeChange.get(persistentUuid); + const afterInstance = instancesByUuidAfterChange.get(persistentUuid); + // Only flash edited instances: an instance appearing or disappearing + // (undo/redo of an addition or deletion) is seen on the canvas, and + // flashing all its rows would be noise. + if (!beforeInstance || !afterInstance) return; + new Set([ + ...Object.keys(beforeInstance), + ...Object.keys(afterInstance), + ]).forEach(key => { + if ( + JSON.stringify(beforeInstance[key]) !== + JSON.stringify(afterInstance[key]) + ) { + const fieldId = serializedInstanceKeyToPropertyFieldId[key]; + if (fieldId) changedFieldIds.add(fieldId); + } + }); + }); + if (changedFieldIds.size === 0) return; + + // A change made from a panel is only visible there: open the + // properties panel to show it. A change made on the canvas is already + // visible: don't open the panel (but still flash if it is open). + const { editorDisplay } = this; + if (!editorDisplay) return; + if (changeContext && changeContext.source === 'panel') { + editorDisplay.ensureEditorVisible('properties'); + } else if (!editorDisplay.isEditorVisible('properties')) { + return; + } + + // Wait for the properties panel to (re-)render with the new values + // before flashing its rows. + requestAnimationFrame(() => { + requestAnimationFrame(() => { + changedFieldIds.forEach(fieldId => { + // Field ids can contain spaces: use an attribute selector. + const element = containerElement.querySelector(`[id="${fieldId}"]`); + if (!element) return; + element.classList.remove('undo-redo-property-flash'); + // Force a reflow so re-adding the class restarts the animation. + void element.offsetWidth; + element.classList.add('undo-redo-property-flash'); + }); + }); + }); + }; + /** * After an undo/redo, make sure its effect can be seen - otherwise the * undo/redo feels like it did nothing: @@ -3291,7 +3418,8 @@ export default class SceneEditor extends React.Component { */ _revealHistoryChanges = ( valueBeforeChange: ?Object, - valueAfterChange: ?Object + valueAfterChange: ?Object, + changeContext: ?{ source: 'canvas' | 'panel' } ) => { const { editorDisplay } = this; if (!editorDisplay) return; @@ -3319,6 +3447,13 @@ export default class SceneEditor extends React.Component { removedInstances, } = diffInstancesSnapshots(beforeChange.instances, afterChange.instances); + this._flashChangedInstancePropertyRows( + beforeChange.instances, + afterChange.instances, + changedOrAddedPersistentUuids, + changeContext + ); + const changedInstances = changedOrAddedPersistentUuids .map(persistentUuid => getInstanceInLayoutWithPersistentUuid( @@ -3572,7 +3707,9 @@ export default class SceneEditor extends React.Component { this.setState({ history: saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ), }), }} @@ -3911,7 +4048,9 @@ export default class SceneEditor extends React.Component { this.setState({ history: saveCompositeToHistory( this.state.history, - this._getHistoryTargets() + this._getHistoryTargets(), + undefined, + { source: 'panel' } ), }); this.openSceneProperties(false);