WindowsCommunityToolkit
WindowsCommunityToolkit copied to clipboard
Expression animation does not work after update the Microsoft.Toolkit.Uwp.UI.Animations nuget package
Describe the bug
After update the Microsoft.Toolkit.Uwp.UI.Animations
nuget package. The expression animation does not work.
Regression
7.0.2
Reproducible in sample app?
- [ ] This bug can be reproduced in the sample app.
Steps to reproduce
I create a bug demo here: https://github.com/h82258652/ToolkitExpressionReferenceBugDemo The 7.0.2 version is working and the 7.1.2 version is not working.
Expected behavior
The 7.1.2 version should work like the 7.0.2 version.
Screenshots
No response
Windows Build Number
- [ ] Windows 10 1809 (Build 17763)
- [ ] Windows 10 1903 (Build 18362)
- [ ] Windows 10 1909 (Build 18363)
- [ ] Windows 10 2004 (Build 19041)
- [ ] Windows 10 20H2 (Build 19042)
- [ ] Windows 10 21H1 (Build 19043)
- [X] Windows 11 21H2 (Build 22000)
- [ ] Other (specify)
Other Windows Build number
No response
App minimum and target SDK version
- [ ] Windows 10, version 1809 (Build 17763)
- [ ] Windows 10, version 1903 (Build 18362)
- [ ] Windows 10, version 1909 (Build 18363)
- [ ] Windows 10, version 2004 (Build 19041)
- [ ] Other (specify)
Other SDK version
No response
Visual Studio Version
2022
Visual Studio Build Number
4.8.04161
Device form factor
Desktop
Nuget packages
7.0.2 7.1.2
Additional context
No response
Help us help you
No.
Hello h82258652, thank you for opening an issue with us!
I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌
This issue has been marked as "needs attention 👋" due to no activity for 15 days. Please triage the issue so the fix can be established.
The link provided for the bug didn't work for me, Can you provide more info on the issue
Oh, sorry. I forget to make it public. It should be works now. Could you try again? Thanks.
Thanks for supply the repro @h82258652.
After some investigation I managed to find the commit that introduced this issue: fa6a3ed9f4de06a275fb4f2046da04ce3aef592f.
I'll continue debugging this on Monday. @arcadiogarcia, this is code you added in #4183, any quick insights you can provide in the meantime?
Haven't tried running the code yet, but I'm pretty sure that the issue is that my fix didn't take into consideration the pattern where CreateConstantVector3 (or some of the other equivalent methods) is used - creating a Vector3 with a ParamName predetermined at its constructor - so then when CreateExpressionAnimationFromNode invokes ClearReferenceInfo it clears the user-given property name and replaces it with a procedural id which does no longer match the id of the Vector3 parameter.
If we all agree that
- The ParamName generated procedurally to refer to "anonymous" nodes shouldn't be reused across runs, and the best way to do that is to wipe them clean right before generating an expression string
- We do want to preserve the ParamName for the nodes that where named by the user, not only to guarantee
then I would propose the following fix
- We create another field under ExpressionNode to hold the ephemeral text that is actually used to refer to the node in the final expressionstring, called nodeName or something like that
- ClearReferenceInfo wipes that new field (instead of ParamName)
- ParamName is still set (and only set when) the user creates a named node (e.g. via CreateExpressionAnimationFromNode) explicitly
- EnsureReferenceInfo populates nodeName using the same logic currently being used to populate ParamName, unless ParamName already has a value, in that case we just copy it
That way we still wipe the name of anonymous nodes, while preserving user-named parameters. Thoughts?
@arcadiogarcia Thanks, I think I understand the issue and why it's happening now. I'll give the proposed fix a try and report back if I hit any issues.
@Arlodotexe Cool! And sorry for the delay, I don't check my github notifications very often, feel free to ping me here or by other channels if I can be of help.