-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Update spine doc for 1.4 #2501
base: dev/1.4
Are you sure you want to change the base?
Update spine doc for 1.4 #2501
Conversation
Opt animator doc
fix: infinite loop when transition to exit but no entry
WalkthroughThis pull request involves comprehensive updates to the Chinese documentation for Spine animations in the Galacean engine. The changes span multiple documentation files and example scripts, focusing on clarifying terminology, updating configuration methods, and streamlining the process of creating and managing Spine animation entities. Key modifications include renaming sections, updating code examples, and introducing new methods for instantiating Spine resources. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/1.4 #2501 +/- ##
===========================================
+ Coverage 68.50% 68.53% +0.03%
===========================================
Files 956 956
Lines 100007 99981 -26
Branches 8545 8549 +4
===========================================
+ Hits 68506 68519 +13
+ Misses 31245 31206 -39
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
🧹 Nitpick comments (10)
examples/spine-physics.ts (1)
37-37
: Replace single quotes with double quotes as per code style guidelinesAccording to the static analysis hint, the string
'wind-idle'
should use double quotes to conform to the project's code style.Apply this diff to fix the issue:
- spine.defaultConfig.animationName = 'wind-idle'; + spine.defaultConfig.animationName = "wind-idle";🧰 Tools
🪛 eslint
[error] 37-37: Replace
'wind-idle'
with"wind-idle"
(prettier/prettier)
examples/spine-animation.ts (1)
26-26
: Remove unnecessary trailing commaStatic analysis indicates that there is an unnecessary trailing comma after the last property in the object literal. Please remove it to comply with the project's code style guidelines.
Apply this diff to fix the issue:
.load({ url: "https://mdn.alipayobjects.com/huamei_kz4wfo/uri/file/as/2/kz4wfo/4/mp/qGISZ7QTJFkEL0Qx/spineboy/spineboy.json", - type: "spine", + type: "spine" })🧰 Tools
🪛 eslint
[error] 26-26: Delete
,
(prettier/prettier)
examples/spine-change-attachment.ts (1)
29-33
: Use double quotes for string literalsFor consistency with TypeScript conventions and to align with the static analysis requirements, use double quotes for string literals.
- spine.defaultConfig.skinName = 'full-skins/girl'; - spine.defaultConfig.animationName = 'idle'; + spine.defaultConfig.skinName = "full-skins/girl"; + spine.defaultConfig.animationName = "idle";🧰 Tools
🪛 eslint
[error] 32-32: Replace
'full-skins/girl'
with"full-skins/girl"
(prettier/prettier)
[error] 33-33: Replace
'idle'
with"idle"
(prettier/prettier)
examples/spine-full-skin-change.ts (1)
35-36
: Use double quotes for string literalsFor consistency with TypeScript conventions and to align with the static analysis requirements, use double quotes for string literals.
- spine.defaultConfig.skinName = 'full-skins/girl'; - spine.defaultConfig.animationName = 'idle'; + spine.defaultConfig.skinName = "full-skins/girl"; + spine.defaultConfig.animationName = "idle";🧰 Tools
🪛 eslint
[error] 35-35: Replace
'full-skins/girl'
with"full-skins/girl"
(prettier/prettier)
[error] 36-36: Replace
'idle'
with"idle"
(prettier/prettier)
examples/spine-mix-and-match.ts (1)
35-36
: Use double quotes for string literalsFor consistency with TypeScript conventions and to align with the static analysis requirements, use double quotes for string literals.
- spine.defaultConfig.skinName = 'full-skins/girl'; - spine.defaultConfig.animationName = 'idle'; + spine.defaultConfig.skinName = "full-skins/girl"; + spine.defaultConfig.animationName = "idle";🧰 Tools
🪛 eslint
[error] 35-35: Replace
'full-skins/girl'
with"full-skins/girl"
(prettier/prettier)
[error] 36-36: Replace
'idle'
with"idle"
(prettier/prettier)
docs/zh/graphics/2D/spine/other.md (2)
9-15
: Enhance the upgrade instructions with version compatibility information.While the upgrade instructions are clear, consider adding a compatibility matrix or version requirements to help users understand which versions of Spine are supported in editor version 1.4.
22-25
: Consider providing specific performance metrics for atlas optimization.The performance recommendations about atlas textures could be enhanced with:
- Recommended maximum atlas size
- Quantitative impact on loading times
- Example scenarios showing before/after optimization
docs/zh/graphics/2D/spine/example.md (2)
16-17
: Fix grammatical issues in template descriptions.Remove the redundant "通过" in these sentences:
-该模板通过展示了如何通过 setAnimation 与 addAnimation API 来编排 spine 动画队列: +该模板展示了如何通过 setAnimation 与 addAnimation API 来编排 spine 动画队列: -该模板通过展示了 Spine 动画如何设置过渡以及不同轨道之间的动画混合: +该模板展示了 Spine 动画如何设置过渡以及不同轨道之间的动画混合:Also applies to: 21-22
17-17
: Add alt text to template images for accessibility.The template images are missing descriptive alt text. Consider adding more descriptive alt text to improve accessibility:
-<img src="..." alt="spine-animation" /> +<img src="..." alt="Spine animation control template showing animation queue management" /> -<img src="..." alt="spine-mix-blend" /> +<img src="..." alt="Spine animation transition and blending template demonstrating track mixing" /> -<img src="..." alt="mix-and-match" /> +<img src="..." alt="Spine mix-and-match template showing skin combination capabilities" /> -<img src="..." alt="spine-dynamic-change" /> +<img src="..." alt="Spine dynamic local skinning template demonstrating attachment replacement" />Also applies to: 22-22, 27-27, 32-32
docs/zh/graphics/2D/spine/editor.md (1)
130-130
: Add more details about PremultiplyAlpha parameter.The description of
PremultiplyAlpha
parameter could be enhanced with:
- When to enable/disable this option
- Impact on rendering quality
- Common troubleshooting scenarios
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/zh/graphics/2D/spine/editor.md
(2 hunks)docs/zh/graphics/2D/spine/example.md
(1 hunks)docs/zh/graphics/2D/spine/other.md
(1 hunks)docs/zh/graphics/2D/spine/overview.md
(1 hunks)docs/zh/graphics/2D/spine/runtime.md
(6 hunks)examples/spine-animation.ts
(2 hunks)examples/spine-change-attachment.ts
(2 hunks)examples/spine-follow-shoot.ts
(3 hunks)examples/spine-full-skin-change.ts
(2 hunks)examples/spine-mix-and-match.ts
(2 hunks)examples/spine-performance.ts
(3 hunks)examples/spine-physics.ts
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/zh/graphics/2D/spine/overview.md
🧰 Additional context used
🪛 LanguageTool
docs/zh/graphics/2D/spine/editor.md
[uncategorized] ~136-~136: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...ild/)流程,导出编辑器项目。
下一章节:[在代码中使用](/docs/graphics/2D/spine/runtim...
(wa5)
docs/zh/graphics/2D/spine/example.md
[uncategorized] ~21-~21: 您的意思是"设置"过度""?
Context: ..." /> 动画过渡与混合 该模板通过展示了 Spine 动画如何设置过渡以及不同轨道之间的动画混合: <img src="https://mdn.ali...
(DU3_DU4)
🪛 eslint
examples/spine-animation.ts
[error] 26-26: Delete ,
(prettier/prettier)
examples/spine-change-attachment.ts
[error] 26-26: Delete ,
(prettier/prettier)
[error] 32-32: Replace 'full-skins/girl'
with "full-skins/girl"
(prettier/prettier)
[error] 33-33: Replace 'idle'
with "idle"
(prettier/prettier)
examples/spine-mix-and-match.ts
[error] 29-29: Delete ,
(prettier/prettier)
[error] 35-35: Replace 'full-skins/girl'
with "full-skins/girl"
(prettier/prettier)
[error] 36-36: Replace 'idle'
with "idle"
(prettier/prettier)
examples/spine-full-skin-change.ts
[error] 29-29: Delete ,
(prettier/prettier)
[error] 35-35: Replace 'full-skins/girl'
with "full-skins/girl"
(prettier/prettier)
[error] 36-36: Replace 'idle'
with "idle"
(prettier/prettier)
examples/spine-follow-shoot.ts
[error] 26-26: Delete ,
(prettier/prettier)
[error] 50-51: Replace ·····);⏎············const·targetBone·=·skeleton.findBone('crosshair'
with ·······const·targetBone·=·skeleton.findBone("crosshair"
(prettier/prettier)
[error] 52-52: Insert ··
(prettier/prettier)
[error] 53-53: Insert ··
(prettier/prettier)
[error] 54-54: Insert ··
(prettier/prettier)
[error] 55-55: Insert ··
(prettier/prettier)
[error] 56-56: Insert ··
(prettier/prettier)
[error] 57-57: Insert ··
(prettier/prettier)
examples/spine-physics.ts
[error] 37-37: Replace 'wind-idle'
with "wind-idle"
(prettier/prettier)
[error] 51-52: Replace ············);⏎·······
with ·········
(prettier/prettier)
🪛 Markdownlint (0.37.0)
docs/zh/graphics/2D/spine/example.md
14-14: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
19-19: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
24-24: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
29-29: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/zh/graphics/2D/spine/runtime.md
74-74: null
Bare URL used
(MD034, no-bare-urls)
75-75: null
Bare URL used
(MD034, no-bare-urls)
76-76: null
Bare URL used
(MD034, no-bare-urls)
39-39: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
95-95: null
Bare URL used
(MD034, no-bare-urls)
96-96: null
Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (22.x, windows-latest)
- GitHub Check: e2e (22.x)
- GitHub Check: codecov
🔇 Additional comments (10)
examples/spine-performance.ts (4)
6-6
: Removed unused importEntity
The import of
Entity
has been removed, which is appropriate since it's no longer used in the code.
18-18
: Verify camera settings after position changeThe camera position has been changed to
new Vector3(0, 0, 50)
. Ensure that the camera's near and far clip planes are appropriately set to accommodate this new position and that the scene renders as expected.
27-29
: Simplified resource instantiation and component retrievalThe spine entity is now instantiated directly from
resource.instantiate()
, and the component is retrieved usinggetComponent(SpineAnimationRenderer)
. This simplifies the code and is an appropriate change.
33-33
: Verify positioning of cloned spine entitiesThe new calculation for positioning clones adjusts their placement in the scene. Verify that this change achieves the desired layout of the spine entities.
examples/spine-animation.ts (3)
6-6
: Removed unused importEntity
The import of
Entity
has been removed, which is appropriate since it's no longer used in the code.
21-21
: Verify camera settings after position changeThe camera position has been changed to
new Vector3(0, 0, 20)
. Ensure that the camera's near and far clip planes are appropriately set to accommodate this new position and that the scene renders as expected.
29-31
: Simplified spine entity instantiation and positioningThe spine entity is now instantiated directly from
spineResource.instantiate()
, positioned at(0, -1.8, 0)
, and theSpineAnimationRenderer
component is retrieved appropriately. This streamlines the code and is appropriate.examples/spine-change-attachment.ts (1)
21-21
: LGTM! Camera position adjustmentThe camera position at z=20 units is well-suited for 2D content visualization and aligns with the standardized camera setup across examples.
examples/spine-follow-shoot.ts (2)
48-52
: LGTM! Coordinate system normalizationThe z-coordinate and vertical offset adjustments align with the normalized coordinate system used across examples.
🧰 Tools
🪛 eslint
[error] 50-51: Replace
·····);⏎············const·targetBone·=·skeleton.findBone('crosshair'
with·······const·targetBone·=·skeleton.findBone("crosshair"
(prettier/prettier)
[error] 52-52: Insert
··
(prettier/prettier)
54-59
: LGTM! Simplified scaling logicThe scaling values have been simplified to use ±1 instead of ±0.05, making the intent clearer.
🧰 Tools
🪛 eslint
[error] 54-54: Insert
··
(prettier/prettier)
[error] 55-55: Insert
··
(prettier/prettier)
[error] 56-56: Insert
··
(prettier/prettier)
[error] 57-57: Insert
··
(prettier/prettier)
[error] 58-58: Insert
··
(prettier/prettier)
[error] 59-59: Insert
··
(prettier/prettier)
```typescript | ||
class extends YourAmazingScript { | ||
async onStart() | ||
// 加载额外的图集文件 | ||
const extraAtlas = await this.engine.resourceManager.load('/extra.atlas') as TextureAtlas; | ||
const { skeleton } = this.entity.getComponent(SpineAnimationRenderer); | ||
// 待替换附件所在的插槽 | ||
const slot = skeleton.findSlot(slotName); | ||
// 用于创建新附件的图集区域 | ||
const region = extraAtlas.findRegion(regionName); | ||
// 基于原本的附件进行克隆出新附件,新附件的图集区域来自于额外的图集文件 | ||
const clone = this.cloneAttachmentWithRegion(slot.attachment, region); | ||
// 替换附件 | ||
slot.attachment = clone; | ||
} |
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.
🛠️ Refactor suggestion
Add error handling to the dynamic atlas loading example.
The code example for dynamic atlas loading should include error handling:
async onStart() {
+ try {
// 加载额外的图集文件
const extraAtlas = await this.engine.resourceManager.load('/extra.atlas') as TextureAtlas;
+ if (!extraAtlas) {
+ throw new Error('Failed to load extra atlas');
+ }
const { skeleton } = this.entity.getComponent(SpineAnimationRenderer);
// 待替换附件所在的插槽
const slot = skeleton.findSlot(slotName);
+ if (!slot) {
+ throw new Error(`Slot ${slotName} not found`);
+ }
// 用于创建新附件的图集区域
const region = extraAtlas.findRegion(regionName);
+ if (!region) {
+ throw new Error(`Region ${regionName} not found in extra atlas`);
+ }
// 基于原本的附件进行克隆出新附件,新附件的图集区域来自于额外的图集文件
const clone = this.cloneAttachmentWithRegion(slot.attachment, region);
// 替换附件
slot.attachment = clone;
+ } catch (error) {
+ console.error('Failed to load and replace attachment:', error);
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```typescript | |
class extends YourAmazingScript { | |
async onStart() | |
// 加载额外的图集文件 | |
const extraAtlas = await this.engine.resourceManager.load('/extra.atlas') as TextureAtlas; | |
const { skeleton } = this.entity.getComponent(SpineAnimationRenderer); | |
// 待替换附件所在的插槽 | |
const slot = skeleton.findSlot(slotName); | |
// 用于创建新附件的图集区域 | |
const region = extraAtlas.findRegion(regionName); | |
// 基于原本的附件进行克隆出新附件,新附件的图集区域来自于额外的图集文件 | |
const clone = this.cloneAttachmentWithRegion(slot.attachment, region); | |
// 替换附件 | |
slot.attachment = clone; | |
} | |
class extends YourAmazingScript { | |
async onStart() { | |
try { | |
// 加载额外的图集文件 | |
const extraAtlas = await this.engine.resourceManager.load('/extra.atlas') as TextureAtlas; | |
if (!extraAtlas) { | |
throw new Error('Failed to load extra atlas'); | |
} | |
const { skeleton } = this.entity.getComponent(SpineAnimationRenderer); | |
// 待替换附件所在的插槽 | |
const slot = skeleton.findSlot(slotName); | |
if (!slot) { | |
throw new Error(`Slot ${slotName} not found`); | |
} | |
// 用于创建新附件的图集区域 | |
const region = extraAtlas.findRegion(regionName); | |
if (!region) { | |
throw new Error(`Region ${regionName} not found in extra atlas`); | |
} | |
// 基于原本的附件进行克隆出新附件,新附件的图集区域来自于额外的图集文件 | |
const clone = this.cloneAttachmentWithRegion(slot.attachment, region); | |
// 替换附件 | |
slot.attachment = clone; | |
} catch (error) { | |
console.error('Failed to load and replace attachment:', error); | |
} | |
} | |
} |
cloneAttachmentWithRegion( | ||
attachment: RegionAttachment | MeshAttachment | Attachment, | ||
atlasRegion: TextureAtlasRegion, | ||
): Attachment { | ||
let newAttachment: RegionAttachment | MeshAttachment; | ||
switch (attachment.constructor) { | ||
case RegionAttachment: | ||
newAttachment = attachment.copy() as RegionAttachment; | ||
newAttachment.region = atlasRegion; | ||
newAttachment.updateRegion(); | ||
break; | ||
case MeshAttachment: | ||
const meshAttachment = attachment as MeshAttachment; | ||
newAttachment = meshAttachment.newLinkedMesh(); | ||
newAttachment.region = atlasRegion; | ||
newAttachment.updateRegion(); | ||
break; | ||
default: | ||
return attachment.copy(); | ||
} | ||
return newAttachment; | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Add type checking and memory management to attachment cloning.
The attachment cloning method should include proper type checking and memory cleanup:
cloneAttachmentWithRegion(
attachment: RegionAttachment | MeshAttachment | Attachment,
atlasRegion: TextureAtlasRegion,
): Attachment {
+ if (!attachment) {
+ throw new Error('Attachment is required');
+ }
+ if (!atlasRegion) {
+ throw new Error('Atlas region is required');
+ }
let newAttachment: RegionAttachment | MeshAttachment;
switch (attachment.constructor) {
case RegionAttachment:
newAttachment = attachment.copy() as RegionAttachment;
newAttachment.region = atlasRegion;
newAttachment.updateRegion();
break;
case MeshAttachment:
const meshAttachment = attachment as MeshAttachment;
+ // Ensure old mesh is properly disposed
+ if (meshAttachment.parentMesh) {
+ meshAttachment.parentMesh.dispose();
+ }
newAttachment = meshAttachment.newLinkedMesh();
newAttachment.region = atlasRegion;
newAttachment.updateRegion();
break;
default:
+ console.warn(`Unsupported attachment type: ${attachment.constructor.name}`);
return attachment.copy();
}
return newAttachment;
}
Committable suggestion skipped: line range outside the PR's diff.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit
Release Notes for Spine Animation Documentation and Examples
Documentation Updates
New Features
PremultiplyAlpha
configuration option for Spine animationsinstantiate()
for creating Spine animation entitiesPerformance Improvements
Breaking Changes
addComponent(SpineAnimationRenderer)
methoddefaultState
withdefaultConfig
in configuration