Skip to content

Fix drag adjustment#618

Merged
MakinoharaShoko merged 9 commits into
OpenWebGAL:devfrom
boomwwww:fix-drag-adjustment
Jun 6, 2026
Merged

Fix drag adjustment#618
MakinoharaShoko merged 9 commits into
OpenWebGAL:devfrom
boomwwww:fix-drag-adjustment

Conversation

@boomwwww

@boomwwww boomwwww commented Jun 5, 2026

Copy link
Copy Markdown
Member

简要介绍

一些解释

  • 关于文件提交的时机,先前,通过一个单独的函数syncCommandToFile进行提交,然后通知其他组件,流程繁琐;现在,改为在调整结束时,通过EffectEditor组件的submit进行提交(EffectEditor第535行)
  • 获取立绘宽高错误的问题,与SetFtoChangeF函数有关(现在位于dragStartUtils.ts第72行),先前,只是简单找出了第一个符合要求的语句;现在,改为了寻找在本语句前最后一个符合要求的语句(建议测试的时候测一下边界)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the TransformableBox component and integrates it into the EffectEditor using React Portals, controlled by a new manual switch, along with corresponding localization updates. The review feedback identifies several critical issues: a potential runtime crash in EffectEditor if the portal target is missing, stale state bugs in TransformableBox due to missing dependencies in useEffect and useCallback, a failure to update the frame after asynchronously fetching asset sizes, potential UI jitter during dragging, and a typo in the onRotate event handler (beforeRotation instead of beforeRotate) that breaks rotation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +505 to +539
{createPortal(
<TransformableBox
parent={document.getElementById('gamePreviewControl') as HTMLElement}
sentenceInfo={{
scenePath: props.targetPath,
lineNumber: props.index,
lineContent: sentenceToRawLine(props.sentence),
lineSentence: props.sentence,
}}
onDragging={(transform) => {
if (transform.position) {
if (transform.position.x !== undefined) {
updateField('x', transform.position.x);
}
if (transform.position.y !== undefined) {
updateField('y', transform.position.y);
}
}
if (transform.scale) {
if (transform.scale.x !== undefined) {
updateField('scaleX', transform.scale.x);
}
if (transform.scale.y !== undefined) {
updateField('scaleY', transform.scale.y);
}
}
updateField('rotation', transform.rotation);
update();
}}
onDragEnd={() => {
submit();
}}
/>,
document.getElementById('gamePreviewControl') as HTMLElement,
)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

createPortal 中直接使用 document.getElementById('gamePreviewControl') 作为容器,如果该 DOM 元素在渲染时不存在(例如在组件卸载、热重载或侧边栏未挂载时),会导致 React 抛出运行时崩溃错误。此外,多次调用 document.getElementById 效率较低。

建议先获取该元素并进行空值校验,确保安全后再渲染 Portal。

      {(() => {
        const container = document.getElementById('gamePreviewControl');
        return container ? createPortal(
          <TransformableBox
            parent={container}
            sentenceInfo={{
              scenePath: props.targetPath,
              lineNumber: props.index,
              lineContent: sentenceToRawLine(props.sentence),
              lineSentence: props.sentence,
            }}
            onDragging={(transform) => {
              if (transform.position) {
                if (transform.position.x !== undefined) {
                  updateField('x', transform.position.x);
                }
                if (transform.position.y !== undefined) {
                  updateField('y', transform.position.y);
                }
              }
              if (transform.scale) {
                if (transform.scale.x !== undefined) {
                  updateField('scaleX', transform.scale.x);
                }
                if (transform.scale.y !== undefined) {
                  updateField('scaleY', transform.scale.y);
                }
              }
              updateField('rotation', transform.rotation);
              update();
            }}
            onDragEnd={() => {
              submit();
            }}
          />,
          container,
        ) : null;
      })()}

Comment on lines 70 to 94
useEffect(() => {
function handlePixiSyncCommand(event: {
targetPath: string;
lineNumber: number;
lineContent: string;
lineSentence: ISentence | null;
}) {
if (event.lineContent.startsWith('changeFigure') && !/changeFigure:\s*none\b/.test(event.lineContent)) {
ChangeFigure(event);
} else if (event.lineContent.startsWith('setTransform')) {
SetTransform(event);
} else {
setIsDisplay(false);
return;
}
}
eventBus.on('editor:pixi-sync-command', handlePixiSyncCommand);
return () => {
eventBus.off('editor:pixi-sync-command', handlePixiSyncCommand);
infoRef.current = {
sentence: sentenceInfo,
transformObj: parseFigureCommand(sentenceInfo.lineContent).transformObj,
figure: {
fileName: '',
type: '',
direction: '',
width: 0,
height: 0,
},
};
getFileNameAndDirection(sentenceInfo).then(({ fileName, direction }) => {
const directory = sentenceInfo.lineSentence.commandRaw === 'changeBg' ? 'background' : 'figure';
getSize(directory, fileName, sentenceInfo.scenePath).then(([width, height]) => {
infoRef.current!.figure = {
fileName,
type: fileName.endsWith('.json') ? 'json' : 'image',
direction,
width,
height,
};
});
});
}, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

这里存在两个严重问题:

  1. useEffect 的依赖项数组为空 [],导致当 sentenceInfo 发生变化(例如用户切换句子或编辑内容)时,infoRef.current 不会更新,从而产生状态过期(Stale State)的 Bug。
  2. getSize 是异步获取尺寸的,但在其 .then 回调中更新了 infoRef.current!.figure 后,并没有调用 updateFrame() 和更新 remountKey。这会导致拖拽框在初始渲染时尺寸为 0 或不正确,且在尺寸获取成功后无法重新计算和渲染。

建议:

  • sentenceInfo 的关键属性和 updateFrame 放入依赖项数组。
  • 在异步回调成功后调用 updateFrame() 并更新 remountKey
  • 使用 isMounted 标记防止组件卸载后更新状态导致的内存泄漏。
  • 增加对 Promise 的错误捕获(.catch)。
  useEffect(() => {
    let isMounted = true;
    infoRef.current = {
      sentence: sentenceInfo,
      transformObj: parseFigureCommand(sentenceInfo.lineContent).transformObj,
      figure: {
        fileName: '',
        type: '',
        direction: '',
        width: 0,
        height: 0,
      },
    };
    getFileNameAndDirection(sentenceInfo).then(({ fileName, direction }) => {
      if (!isMounted) return;
      const directory = sentenceInfo.lineSentence.commandRaw === 'changeBg' ? 'background' : 'figure';
      getSize(directory, fileName, sentenceInfo.scenePath).then(([width, height]) => {
        if (!isMounted) return;
        infoRef.current!.figure = {
          fileName,
          type: fileName.endsWith('.json') ? 'json' : 'image',
          direction,
          width,
          height,
        };
        updateFrame();
        setRemountKey((prev) => prev + 1);
      }).catch((err) => {
        console.error('Failed to get asset size:', err);
      });
    }).catch((err) => {
      console.error('Failed to get file name and direction:', err);
    });
    return () => {
      isMounted = false;
    };
  }, [sentenceInfo.scenePath, sentenceInfo.lineNumber, sentenceInfo.lineContent, updateFrame]);

Comment on lines +136 to +165
useEffect(() => {
const handleSyncDragger = (transform: {
x: number;
y: number;
scaleX: number;
scaleY: number;
rotation: number;
}) => {
if (!infoRef.current) {
return;
}
});
};
infoRef.current.transformObj = {
position: {
x: transform.x,
y: transform.y,
},
scale: {
x: transform.scaleX,
y: transform.scaleY,
},
rotation: transform.rotation,
};
updateFrame();
refresh();
};
eventBus.on('editor:sync-dragger', handleSyncDragger);
return () => {
eventBus.off('editor:sync-dragger', handleSyncDragger);
};
}, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

在用户主动进行拖拽、缩放或旋转时,editor:sync-dragger 事件仍会被触发并执行 updateFrame()。由于控制坐标与预览坐标转换时存在 Math.round 四舍五入误差,这会导致在拖拽过程中拖拽框发生抖动(Jitter)或跳跃。

建议引入一个 isDraggingRef 标识(例如 const isDraggingRef = useRef(false);),在拖拽/缩放/旋转开始时设为 true,结束时设为 false。在 handleSyncDragger 中,如果正在拖拽则直接忽略同步事件。

  useEffect(() => {
    const handleSyncDragger = (transform: {
      x: number;
      y: number;
      scaleX: number;
      scaleY: number;
      rotation: number;
    }) => {
      if (isDraggingRef.current) {
        return;
      }
      if (!infoRef.current) {
        return;
      }
      infoRef.current.transformObj = {
        position: {
          x: transform.x,
          y: transform.y,
        },
        scale: {
          x: transform.scaleX,
          y: transform.scaleY,
        },
        rotation: transform.rotation,
      };
      updateFrame();
      refresh();
    };
    eventBus.on('editor:sync-dragger', handleSyncDragger);
    return () => {
      eventBus.off('editor:sync-dragger', handleSyncDragger);
    };
  }, []);

Comment on lines +248 to +250
onRotate={({ beforeRotation, drag }) => {
const translate = drag.beforeTranslate as [number, number];
tempState.set({ ...tempState.value, rotate: beforeRotation, translate });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

onRotate 事件中,解构的属性名称错误。react-moveable 的事件对象中并没有 beforeRotation 属性,正确的属性名称是 beforeRotate。使用 beforeRotation 会导致其值为 undefined,从而使旋转功能完全失效。

Suggested change
onRotate={({ beforeRotation, drag }) => {
const translate = drag.beforeTranslate as [number, number];
tempState.set({ ...tempState.value, rotate: beforeRotation, translate });
onRotate={({ beforeRotate, drag }) => {
const translate = drag.beforeTranslate as [number, number];
tempState.set({ ...tempState.value, rotate: beforeRotate, translate });

Comment on lines +97 to +120
const updateFrame = useCallback(() => {
const position = infoRef.current?.transformObj.position
? convertPreviewToControl(infoRef.current.transformObj.position, parent)
: { x: 0, y: 0 };

const SetTransform = (event: {
targetPath: string;
lineNumber: number;
lineContent: string;
lineSentence: ISentence | null;
}) => {
if (!event.lineSentence) {
return;
const scaledSize = calculateScaledImageSize(
infoRef.current?.figure.width ?? 0,
infoRef.current?.figure.height ?? 0,
);
const size = convertPreviewToControl({ x: scaledSize.width, y: scaledSize.height }, parent);

frameState.set({
...frameState.value,
translate: [
position.x + ToXOffset(infoRef.current!.figure.direction ?? '', parent, size.x ?? frameState.value.width),
position.y,
],
scale: [infoRef.current!.transformObj.scale.x, infoRef.current!.transformObj.scale.y],
rotate: radiansToDegrees(infoRef.current!.transformObj.rotation),
width: size.x,
height: size.y,
});
tempState.set(frameState.value);
}, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

updateFrame 内部使用了组件的 parent 属性,但其 useCallback 的依赖项数组为空 []。如果 parent 发生变化,updateFrame 将捕获过期的 parent 引用。建议将 parent 添加到依赖项数组中。

  const updateFrame = useCallback(() => {
    const position = infoRef.current?.transformObj.position
      ? convertPreviewToControl(infoRef.current.transformObj.position, parent)
      : { x: 0, y: 0 };

    const scaledSize = calculateScaledImageSize(
      infoRef.current?.figure.width ?? 0,
      infoRef.current?.figure.height ?? 0,
    );
    const size = convertPreviewToControl({ x: scaledSize.width, y: scaledSize.height }, parent);

    frameState.set({
      ...frameState.value,
      translate: [
        position.x + ToXOffset(infoRef.current!.figure.direction ?? '', parent, size.x ?? frameState.value.width),
        position.y,
      ],
      scale: [infoRef.current!.transformObj.scale.x, infoRef.current!.transformObj.scale.y],
      rotate: radiansToDegrees(infoRef.current!.transformObj.rotation),
      width: size.x,
      height: size.y,
    });
    tempState.set(frameState.value);
  }, [parent]);

@MakinoharaShoko MakinoharaShoko merged commit 380d58b into OpenWebGAL:dev Jun 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants