-
-
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
Fix animate property is not effect when reference changed #2383
base: dev/1.4
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,121 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
* @title Animation Reference And Property | ||||||||||||||||||||||||||||||||||||||||||||||||||||
* @category Animation | ||||||||||||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimationClip, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimationColorCurve, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimationFloatCurve, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimationRefCurve, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Animator, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimatorController, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
AnimatorControllerLayer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
BlinnPhongMaterial, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Camera, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Color, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
DirectLight, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Keyframe, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Logger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Material, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
MeshRenderer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
PBRMaterial, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
PrimitiveMesh, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
RenderFace, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Transform, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Vector3, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
WebGLEngine | ||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "@galacean/engine"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
import { OrbitControl } from "@galacean/engine-toolkit"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
import { initScreenshot, updateForE2E } from "./.mockForE2E"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
Logger.enable(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
WebGLEngine.create({ canvas: "canvas" }).then((engine) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
engine.canvas.resizeByClientSize(2); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const scene = engine.sceneManager.activeScene; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const rootEntity = scene.createRootEntity(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
// camera | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const cameraEntity = rootEntity.createChild("camera_node"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
cameraEntity.transform.position = new Vector3(0, 1, 5); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const camera = cameraEntity.addComponent(Camera); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
cameraEntity.addComponent(OrbitControl).target = new Vector3(0, 1, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const lightNode = rootEntity.createChild("light_node"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
lightNode.addComponent(DirectLight).intensity = 0.6; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
lightNode.transform.lookAt(new Vector3(0, 0, 1)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
lightNode.transform.rotate(new Vector3(0, 90, 0)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const material = new BlinnPhongMaterial(engine); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
material.renderFace = RenderFace.Double; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
material.baseColor = new Color(1, 0, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const material2 = new PBRMaterial(engine); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
material2.renderFace = RenderFace.Double; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
material2.baseColor = new Color(0, 1, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const entity = rootEntity.createChild("mesh"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const { transform } = entity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
transform.setPosition(0, 1, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
transform.setRotation(0, 0, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const meshRenderer = entity.addComponent(MeshRenderer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
meshRenderer.mesh = PrimitiveMesh.createCuboid(engine); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
meshRenderer.setMaterial(material); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const animator = entity.addComponent(Animator); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const controller = new AnimatorController(engine); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const layer = new AnimatorControllerLayer("base"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
controller.addLayer(layer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const stateMachine = layer.stateMachine; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const cubeState = stateMachine.addState("material"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const cubeClip = (cubeState.clip = new AnimationClip("material")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const materialCurve = new AnimationRefCurve(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key1 = new Keyframe<Material>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key1.time = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key1.value = material; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key2 = new Keyframe<Material>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key2.time = 1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key2.value = material2; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key3 = new Keyframe<Material>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key3.time = 2; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key3.value = material; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
materialCurve.addKey(key1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
materialCurve.addKey(key2); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
materialCurve.addKey(key3); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+73
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplify Keyframe creation for cleaner code You can streamline the creation of keyframes by initializing them within a loop or using an array, which reduces repetitive code and enhances readability. An example refactor: const materialCurve = new AnimationRefCurve();
- const key1 = new Keyframe<Material>();
- key1.time = 0;
- key1.value = material;
- const key2 = new Keyframe<Material>();
- key2.time = 1;
- key2.value = material2;
- const key3 = new Keyframe<Material>();
- key3.time = 2;
- key3.value = material;
- materialCurve.addKey(key1);
- materialCurve.addKey(key2);
- materialCurve.addKey(key3);
+ const keyframes = [
+ { time: 0, value: material },
+ { time: 1, value: material2 },
+ { time: 2, value: material },
+ ];
+ keyframes.forEach(({ time, value }) => {
+ const key = new Keyframe<Material>();
+ key.time = time;
+ key.value = value;
+ materialCurve.addKey(key);
+ }); Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const colorCurve = new AnimationColorCurve(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key6 = new Keyframe<Color>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key6.time = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key6.value = new Color(1, 0, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key7 = new Keyframe<Color>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key7.time = 1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key7.value = new Color(0.5, 0.5, 0.5, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key8 = new Keyframe<Color>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key8.time = 2; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key8.value = new Color(1, 0, 0, 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
colorCurve.addKey(key6); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
colorCurve.addKey(key7); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
colorCurve.addKey(key8); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
const rotateCurve = new AnimationFloatCurve(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key9 = new Keyframe<number>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key9.time = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key9.value = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
const key10 = new Keyframe<number>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key10.time = 2; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
key10.value = 360; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
rotateCurve.addKey(key9); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
rotateCurve.addKey(key10); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
cubeClip.addCurveBinding("", MeshRenderer, "getMaterial().baseColor", colorCurve); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
cubeClip.addCurveBinding("", MeshRenderer, "setMaterial($value)", "getMaterial()", materialCurve); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verify the correctness of material property binding The method names Consider updating the binding to directly reference the material property: - cubeClip.addCurveBinding("", MeshRenderer, "setMaterial($value)", "getMaterial()", materialCurve);
+ cubeClip.addCurveBinding("", MeshRenderer, "material", materialCurve);
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
cubeClip.addCurveBinding("", Transform, "rotation.y", rotateCurve); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
animator.animatorController = controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
animator.play("material"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
animator.speed = 0.5; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
updateForE2E(engine, 130); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
initScreenshot(engine, camera); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,12 +6,15 @@ | |
import { AnimationCurve } from "./animationCurve/AnimationCurve"; | ||
import { AnimationEvent } from "./AnimationEvent"; | ||
import { AnimationCurveOwner } from "./internal/animationCurveOwner/AnimationCurveOwner"; | ||
import { AnimationPropertyReferenceManager } from "./internal/AnimationPropertyReferenceManager"; | ||
import { KeyframeValueType } from "./Keyframe"; | ||
|
||
/** | ||
* Stores keyframe based animations. | ||
*/ | ||
export class AnimationClip extends EngineObject { | ||
private static _tempReferenceManager: AnimationPropertyReferenceManager; | ||
|
||
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider changing Using a static Apply this diff to fix the issue: - private static _tempReferenceManager: AnimationPropertyReferenceManager;
+ private _tempReferenceManager: AnimationPropertyReferenceManager;
...
- if (!AnimationClip._tempReferenceManager) {
- AnimationClip._tempReferenceManager = new AnimationPropertyReferenceManager();
- }
- AnimationClip._tempReferenceManager.clear();
+ if (!this._tempReferenceManager) {
+ this._tempReferenceManager = new AnimationPropertyReferenceManager();
+ }
+ this._tempReferenceManager.clear();
...
- const curveOwner = curve._getTempCurveOwner(AnimationClip._tempReferenceManager, targetEntity, component);
+ const curveOwner = curve._getTempCurveOwner(this._tempReferenceManager, targetEntity, component); Also applies to: 230-235, 248-248 |
||
/** @internal */ | ||
_curveBindings: AnimationClipCurveBinding[] = []; | ||
|
||
|
@@ -193,7 +196,20 @@ | |
} | ||
|
||
this._length = Math.max(this._length, curveBinding.curve.length); | ||
this._curveBindings.push(curveBinding); | ||
|
||
const curveBindings = this._curveBindings; | ||
const count = curveBindings.length; | ||
const propertyPathLength = curveBinding.property.split(".").length; | ||
curveBinding._pathLength = propertyPathLength; | ||
|
||
const maxPropertyPathLength = count ? curveBindings[count - 1]._pathLength : 0; | ||
if (propertyPathLength > maxPropertyPathLength) { | ||
curveBindings.push(curveBinding); | ||
} else { | ||
let index = count; | ||
while (--index >= 0 && propertyPathLength < curveBindings[index]._pathLength); | ||
curveBindings.splice(index + 1, 0, curveBinding); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -211,6 +227,12 @@ | |
* @param time - The time to sample an animation | ||
*/ | ||
_sampleAnimation(entity: Entity, time: number): void { | ||
if (!AnimationClip._tempReferenceManager) { | ||
AnimationClip._tempReferenceManager = new AnimationPropertyReferenceManager(); | ||
} | ||
|
||
AnimationClip._tempReferenceManager.clear(); | ||
|
||
const { _curveBindings: curveBindings } = this; | ||
for (let i = curveBindings.length - 1; i >= 0; i--) { | ||
const curve = curveBindings[i]; | ||
|
@@ -223,7 +245,7 @@ | |
if (!component) { | ||
continue; | ||
} | ||
const curveOwner = curve._getTempCurveOwner(targetEntity, component); | ||
const curveOwner = curve._getTempCurveOwner(AnimationClip._tempReferenceManager, targetEntity, component); | ||
if (curveOwner && curve.curve.keys.length) { | ||
const value = curveOwner.evaluateValue(curve.curve, time, false); | ||
curveOwner.applyValue(value, 1, false); | ||
|
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.
Avoid assignments within expressions for clarity
Assigning a value within an expression can make the code harder to read and maintain. It's better to separate the assignments into individual statements for improved readability.
Apply this diff to separate the assignments:
Committable suggestion
Tools
Biome