Feat drag adjustment #500
Feat drag adjustment #500
Conversation
Summary of ChangesHello @bysq-2006, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求引入了一项重要的交互式功能，允许用户直接在预览窗口中通过拖拽操作来调整图形元素（如立绘）的位置、缩放和旋转。这一改进旨在提供更直观、高效的视觉编辑体验，并确保所有调整都能实时反映到代码中。为了支持这一功能，项目还进行了核心图形库Pixi.js的重大版本升级和相关API的兼容性调整。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 添加了预览窗口的拖拽调整功能，这是一个非常实用的功能，极大地提升了用户体验。同时，PR 还将 pixi.js 从 v6 升级到了 v8，并适配了新的 API。
代码审查主要集中在新引入的 usePixiApp hook 上。我提出了一些关于代码健壮性、可维护性和性能的建议。主要包括：
- 重构状态管理方式，以更好地适应 React 的生命周期，避免使用模块级可变状态。
- 优化文件更新逻辑，减少不必要的 I/O 开销。
- 修复了处理命令字符串时的一个潜在 bug。
- 通过移除
any类型和魔法数字来增强类型安全和代码可读性。
整体来看，这是一个很棒的功能实现，上述建议旨在使其更加完善。
| let currentFrame: Container | null = null; | ||
| // 用于保存 PIXI App 实例 | ||
| let appInstance: Application | null = null; | ||
| // 用于保存原始指令 | ||
| let originalCommand: string | null = null; | ||
| // 用于保存当前编辑的上下文 | ||
| let commandContext: { path: string, targetPath: string, lineNumber: number } | null = null; |
There was a problem hiding this comment.
这个 hook 中使用了模块级别的 let 变量（currentFrame, appInstance, originalCommand, commandContext）来存储状态。在 React 中，这是一种危险的做法，尤其是在并发模式下，可能会导致竞态条件和难以调试的 bug。模块级变量在所有组件实例之间共享，并且它们的变更不会触发 React 的重新渲染。
建议将这些状态移入 React 的生命周期中管理。
- 对于像
appInstance这样在多次渲染之间保持不变的实例，可以使用useRef。 - 对于需要在组件间共享的状态，可以考虑使用 React Context 或更专业的全局状态管理库（如 Zustand，项目中已在使用）。
例如，可以将这些变量移入 usePixiApp hook 内部，并用 useRef 来管理：
export function usePixiApp(canvasRef: React.RefObject<HTMLCanvasElement>) {
const appRef = useRef<Application | null>(null);
const currentFrameRef = useRef<Container | null>(null);
const originalCommandRef = useRef<string | null>(null);
const commandContextRef = useRef<{ path: string, targetPath: string, lineNumber: number } | null>(null);
// ... 在后续逻辑中使用 ...Ref.current
// ...
}这样做可以确保状态与组件实例的生命周期绑定，避免潜在的副作用。
| } | ||
|
|
||
| // 1. 先用更健壮的正则去掉原始命令中的 -transform={...}（无论有无） | ||
| const commandWithoutTransform = originalCommand.replace(/\s*-transform=\{.*\}/, '').replace(/[\s\r\n;\}]+$/, ''); |
There was a problem hiding this comment.
在 containerToCommandString 函数中，用于移除旧 -transform 参数的正则表达式 /\s*-transform=\{.*}/ 是贪婪的。如果一行命令中有多个 {...} 结构，它会错误地从第一个 { 匹配到最后一个 }，可能移除过多内容。
例如，对于指令 changeFigure -transform={...} -other={...}，这个正则会把 -transform 和 -other 都移除掉。
一个更健壮的方法是使用非贪婪匹配 /\s*-transform=\{.*?\}/，或者更好的方式是解析出命令中除了 transform 之外的所有部分，然后用新的 transform 对象重新构建指令字符串，以确保逻辑的正确性和一致性。
| const commandWithoutTransform = originalCommand.replace(/\s*-transform=\{.*\}/, '').replace(/[\s\r\n;\}]+$/, ''); | |
| const commandWithoutTransform = originalCommand.replace(/\s*-transform=\{[^}]*\}/, '').replace(/[\s\r\n;\}]+$/, ''); |
| async function syncCommandToFile(newCommand: string) { | ||
| if (!commandContext) { | ||
| console.warn('No command context available for sync'); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| // 1. 读取文件内容 - 使用 targetPath (完整路径) | ||
| const response = await axios.get(commandContext.targetPath); | ||
| const fileContent = response.data.toString(); | ||
| const lines = fileContent.split('\n'); | ||
|
|
||
| // 2. 替换指定行 (lineNumber 是从 1 开始的,所以需要 -1) | ||
| const lineIndex = commandContext.lineNumber - 1; | ||
| if (lineIndex >= 0 && lineIndex < lines.length) { | ||
| lines[lineIndex] = newCommand; | ||
| const newFileContent = lines.join('\n'); | ||
|
|
||
| // 3. 保存文件 - 使用 targetPath (完整路径) 传给后端 API | ||
| await api.assetsControllerEditTextFile({ | ||
| path: commandContext.targetPath, // ✅ 使用完整路径 | ||
| textFile: newFileContent | ||
| }); | ||
|
|
||
| // 通知其他组件文件已更新 | ||
| eventBus.emit('drag-update-scene'); | ||
|
|
||
| console.log('Command synced successfully:', newCommand); | ||
| } else { | ||
| console.error('Line number out of range:', commandContext.lineNumber, 'Total lines:', lines.length); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to sync command to file:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
syncCommandToFile 函数的实现方式是读取整个文件，修改其中一行，然后将整个文件内容写回。对于大文件来说，这种方式效率很低，可能会成为性能瓶颈，并导致不佳的用户体验。
如果后端 API 支持，更好的做法是只发送需要修改的行号和新内容，让后端来处理文件的局部更新（类似 patch 操作）。如果后端 API 无法修改，也请考虑这个操作对用户来说是异步且无感知的，避免阻塞 UI。
| } | ||
| })(); | ||
|
|
||
| const handleSyncCommand = (params: any) => { |
There was a problem hiding this comment.
在 handleSyncCommand 函数中，参数 params 的类型被指定为 any，这会绕过 TypeScript 的类型检查，降低代码的可维护性和安全性。
建议为 params 定义一个明确的接口类型，以增强类型安全。
interface SyncCommandParams {
path: string;
targetPath: string;
lineNumber: number;
lineContent: string;
}
const handleSyncCommand = (params: SyncCommandParams) => {
// ...
};同样的问题也存在于 getFigureTransform 的 transformObj 和 handlePointerUp 的 obj 参数。为它们定义明确的类型将使代码更健壮。
| const handleSyncCommand = (params: any) => { | |
| const handleSyncCommand = (params: { path: string, targetPath: string, lineNumber: number, lineContent: string }) => { |
| const directionMap = { | ||
| left: 0.15, | ||
| center: 0.5, | ||
| right: 0.85 | ||
| }; |
There was a problem hiding this comment.
在 getFigureTransform 函数中，directionMap 使用了 0.15, 0.5, 0.85 这样的魔法数字。虽然它们在上下文中可能很清晰，但将它们定义为具名常量可以提高代码的可读性和可维护性。
例如：
const FIGURE_POSITIONS = {
LEFT: 0.15,
CENTER: 0.5,
RIGHT: 0.85,
};
const directionMap = {
left: FIGURE_POSITIONS.LEFT,
center: FIGURE_POSITIONS.CENTER,
right: FIGURE_POSITIONS.RIGHT,
};| center: 0.5, | ||
| right: 0.85 | ||
| }; | ||
| let percentX = directionMap[direction] ?? 0.45; |
There was a problem hiding this comment.
在 getFigureTransform 函数中，当 direction 无效时，percentX 的回退值是 0.45。然而，center 的值是 0.5。这看起来像是一个笔误。如果意图是默认居中，那么回退值应该是 0.5。
| let percentX = directionMap[direction] ?? 0.45; | |
| let percentX = directionMap[direction] ?? 0.5; |
| function createMoveListener(size = 100, color = 0xff9900, alpha = 0.5, strokeColor = 0x000000, strokeWidth = 2) { | ||
| const square = new Graphics(); | ||
| square | ||
| .rect(-size / 2, -size / 2, size, size) | ||
| .fill({ color, alpha }) | ||
| .stroke({ width: strokeWidth, color: strokeColor }); | ||
|
|
||
| square.scale.set(0.8); | ||
| square.eventMode = 'static'; // 启用事件模式（Pixi v7+ 必须） | ||
| square.cursor = 'pointer'; // 鼠标悬停时显示手型（可选） | ||
| (square as any).ismove = false; // 自定义属性，表示是否正在移动 | ||
| square.on('pointerdown', (event) => { | ||
| (square as any).ismove = true; | ||
| }); | ||
| square.on('pointermove', (event) => { | ||
| if ((square as any).ismove && square.parent) { | ||
| // event.global contains the pointer's position in canvas coordinates | ||
| const newPosition = event.global; | ||
| square.parent.x = newPosition.x; | ||
| square.parent.y = newPosition.y; | ||
| } | ||
| }); | ||
| square.on('pointerup', () => handlePointerUp('move', square)); | ||
| square.on('pointerupoutside', () => handlePointerUp('move', square)); | ||
| return square; | ||
| } | ||
|
|
||
| function createZoomListener(size = 100, color = 0x0099ff, alpha = 0.5, strokeColor = 0x000000, strokeWidth = 2) { | ||
| const circle = new Graphics() | ||
| .circle(0, 0, 60) // 半径 60 的圆 | ||
| .stroke({ width: 4, color: 0x66ccff }) // 4px 蓝色描边 | ||
| .fill({ color: 0x000000, alpha: 0 }); // 透明填充 | ||
| circle.scale.set(1.2); | ||
| circle.eventMode = 'static'; // 启用事件模式（Pixi v7+ 必须） | ||
| circle.cursor = 'pointer'; // 鼠标悬停时显示手型（可选） | ||
| (circle as any).iszoom = false; // 自定义属性，表示是否正在缩放 | ||
| circle.on('pointerdown', (event) => { | ||
| (circle as any).iszoom = true; | ||
| }); | ||
| circle.on('pointermove', (event) => { | ||
| if ((circle as any).iszoom && circle.parent) { | ||
| // event.global contains the pointer's position in canvas coordinates | ||
| const newPosition = event.global; | ||
| const dx = newPosition.x - circle.parent.x; | ||
| const dy = newPosition.y - circle.parent.y; | ||
| const distance = Math.sqrt(dx * dx + dy * dy); | ||
| console.log('distance:', distance, size / 2); | ||
| const newScale = distance / (size / 2); | ||
| circle.parent.scale.set(newScale); | ||
| } | ||
| }); | ||
| circle.on('pointerup', () => handlePointerUp('zoom', circle)); | ||
| circle.on('pointerupoutside', () => handlePointerUp('zoom', circle)); | ||
| return circle; | ||
| } | ||
|
|
||
| function createRotateListener(size = 100) { | ||
| const circle = new Graphics() | ||
| .circle(0, 0, size / 4) // 半径 size/2 的圆 | ||
| .stroke({ width: 4, color: 0xffcc00 }) // 4px 黄色描边 | ||
| .fill({ color: 0x000000, alpha: 0 }); | ||
| circle.x = size * 1.2; | ||
| circle.eventMode = 'static'; // 启用事件模式（Pixi v7+ 必须） | ||
| circle.cursor = 'pointer'; // 鼠标悬停时显示手型（可选） | ||
| (circle as any).isrotate = false; // 自定义属性，表示是否正在旋转 | ||
| circle.on('pointerdown', (event) => { | ||
| (circle as any).isrotate = true; | ||
| }); | ||
| circle.on('pointermove', (event) => { | ||
| if ((circle as any).isrotate && circle.parent) { | ||
| const newPosition = event.global; | ||
| const dx = newPosition.x - circle.parent.x; | ||
| const dy = newPosition.y - circle.parent.y; | ||
| const angle = Math.atan2(dy, dx); | ||
| circle.parent.rotation = angle; | ||
| } | ||
| }); | ||
| circle.on('pointerup', () => handlePointerUp('rotate', circle)); | ||
| circle.on('pointerupoutside', () => handlePointerUp('rotate', circle)); | ||
| return circle; | ||
| } |
There was a problem hiding this comment.
createMoveListener, createZoomListener, 和 createRotateListener 这三个函数中包含了大量重复的事件监听器设置代码（如 eventMode, cursor, on('pointerdown') 等）。
为了提高代码的可维护性并减少重复，可以考虑将这部分通用逻辑提取到一个辅助函数中。例如，可以创建一个 createDraggable 函数，它接收特定的移动/缩放/旋转逻辑作为参数，并返回一个配置好通用事件的 Graphics 对象。
| square.scale.set(0.8); | ||
| square.eventMode = 'static'; // 启用事件模式（Pixi v7+ 必须） | ||
| square.cursor = 'pointer'; // 鼠标悬停时显示手型（可选） | ||
| (square as any).ismove = false; // 自定义属性，表示是否正在移动 |
There was a problem hiding this comment.
在 createMoveListener 和其他类似的监听器创建函数中，通过 (square as any).ismove = false; 的方式为 PIXI 对象添加自定义属性。使用 any 类型转换会失去类型安全。
为了更安全地扩展对象，可以定义一个扩展接口：
import { Graphics } from 'pixi.js';
interface DraggableGraphics extends Graphics {
ismove?: boolean;
iszoom?: boolean;
isrotate?: boolean;
}
// ...
(square as DraggableGraphics).ismove = false;这样可以在不完全放弃类型检查的情况下，为对象添加自定义属性。
| const dx = newPosition.x - circle.parent.x; | ||
| const dy = newPosition.y - circle.parent.y; | ||
| const distance = Math.sqrt(dx * dx + dy * dy); | ||
| console.log('distance:', distance, size / 2); |
There was a problem hiding this comment.
代码中存在用于调试的 console.log 语句。这些语句在开发过程中很有用，但在生产环境中应该被移除，以避免向用户控制台输出不必要的信息，并可能暴露内部实现细节。
| console.log('distance:', distance, size / 2); | |
| // console.log('distance:', distance, size / 2); |
3 participants
awa
功能说明
添加了预览窗口拖拽调整功能，允许用户通过拖拽实时调整立绘的位置、大小和旋转。
主要改动
使用方法