-
-
Notifications
You must be signed in to change notification settings - Fork 321
feat: extend transform argument usage #864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @HardyNLee, 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! 此拉取请求旨在增强 WebGAL 引擎的动画控制能力,特别是针对 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个 PR 扩展了 transform 参数的用法,允许在不改变图像 URL 的情况下应用变换,这是一个很好的功能增强。同时,代码中通过提取 registerTimelineAnimation 和 removeTimelineAnimation 函数来重构动画逻辑,减少了重复代码,值得称赞。
不过,在重构过程中,setAnimation、setTempAnimation 和 setTransform 中处理 keep 参数的逻辑似乎引入了一个 bug,导致 keep 功能可能无法按预期工作。此外,在 changeBg 和 changeFigure 中仍然存在一些代码重复,可以进一步优化。
我已经在具体的代码行上提出了一些修改建议,希望能帮助你完善这个功能。
| // eslint-disable-next-line max-params | ||
| export function registerTimelineAnimation( | ||
| animationName: string, | ||
| animationKey: string, | ||
| target: string, | ||
| animationDuration: number, | ||
| writeDefault: boolean, | ||
| keep: boolean, | ||
| keepAnimationStopped: boolean, | ||
| ) { | ||
| setTimeout(() => { | ||
| if (keep && keepAnimationStopped) { | ||
| return; | ||
| } | ||
| WebGAL.gameplay.pixiStage?.stopPresetAnimationOnTarget(target); | ||
| const animationObj: IAnimationObject | null = getAnimationObject( | ||
| animationName, | ||
| target, | ||
| animationDuration, | ||
| writeDefault, | ||
| ); | ||
| if (animationObj) { | ||
| logger.debug(`动画${animationName}作用在${target}`, animationDuration); | ||
| WebGAL.gameplay.pixiStage?.registerAnimation(animationObj, animationKey, target); | ||
| } | ||
| }, 0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
registerTimelineAnimation 函数在处理 keepAnimationStopped 时存在逻辑问题。keepAnimationStopped 是通过值传递的,因此在 setTimeout 的闭包中,它将始终使用初始值(即 false),这会破坏防止重新注册“保留”的动画的预期逻辑。我建议重构此函数,移除 setTimeout 和与 keep 相关的逻辑,将该职责移回调用方。这将修复闭包问题,同时保持核心动画注册逻辑的封装。
export function registerTimelineAnimation(
animationName: string,
animationKey: string,
target: string,
animationDuration: number,
writeDefault: boolean,
) {
WebGAL.gameplay.pixiStage?.stopPresetAnimationOnTarget(target);
const animationObj: IAnimationObject | null = getAnimationObject(
animationName,
target,
animationDuration,
writeDefault,
);
if (animationObj) {
logger.debug(`动画${animationName}作用在${target}`, animationDuration);
WebGAL.gameplay.pixiStage?.registerAnimation(animationObj, animationKey, target);
}
}| registerTimelineAnimation( | ||
| animationName, | ||
| animationKey, | ||
| target, | ||
| animationDuration, | ||
| writeDefault, | ||
| keep, | ||
| keepAnimationStopped, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| registerTimelineAnimation( | ||
| animationName, | ||
| animationKey, | ||
| target, | ||
| animationDuration, | ||
| writeDefault, | ||
| keep, | ||
| keepAnimationStopped, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| registerTimelineAnimation( | ||
| animationName, | ||
| animationKey, | ||
| target, | ||
| animationDuration, | ||
| writeDefault, | ||
| keep, | ||
| keepAnimationStopped, | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
介绍
在
changeBgchangeFigureurl 未更改时,-transform参数会像setTransform命令一样工作相当于以前的
changFigure: stand.webp -id=aaa -transform={"position":{"x":-500}}; setTransform: {"position":{"x":500}} -target=aaa -duration=1500;现在可以写成
changFigure: stand.webp -id=aaa -transform={"position":{"x":-500}}; changFigure: stand.webp -id=aaa -transform={"position":{"x":500}} -duration=1500;这对于一边更改立绘/背景参数 (如 zIndex, live2d 的 motion expression), 一边做变换效果的情况, 可以简写为一行, 同时也减少了很多新手问 为什么我设置(changeFigure)的变换效果没有动画 的情况
因为有
isUrlChanged做隔离, 所以按理说应该不会影响入场退场的逻辑为
changeBgchangeFigure添加了writeDefault参数, 但是keep暂时不加, 我总感觉有点隐式的问题, 等我以后相通了再加这个 PR 实际上与 #863 有一点点重叠, 不过问题不大
编辑器那边等这个 PR 合并后, 再移除掉那个"请用设置效果命令" 的提示