Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Realistic vehicle wheel speed Sandcastle example #7361
Realistic vehicle wheel speed Sandcastle example #7361
Changes from 11 commits
7b36978
a68c1b8
ddc4e14
aa18d5d
8f96193
147c19e
441c2c0
7bf4b9b
adb88ce
28b1cc2
650ba0e
64881e5
000f987
7c7fcac
5921878
61bed63
465a4dc
2c3b590
e3bd3b8
15c98a4
1a20d10
052d810
7f53144
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Only use inline comments for things that are non-obvious. I don't think this comment is needed.
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.
What exactly is an
activeAnimation
can an animation become inactive? And if that happens will it's multiplier setting become out of date?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.
activeAnimations
is an instance ofModelAnimationCollection
, which keeps track of all currently playing animations. Theoretically yes if you remove an animation, and add it back in its multiplier value will be out of sync. There's no way to do this for individual animations at the Entity level though. So I'd say the solution is, if/when we implement that, to setmodelData.multiplier
to undefined every time an animation is added/removed, (in the same way thatModelAnimationCollection
currently keeps track of when the multiplier changes to recompute duration and other things).Or we could just loop over all animations and re-set the multiplier every frame the way it sets all other properties on model every frame.
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.
This property doesn't seem to be used anywhere
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.
It is. See #7361 (comment)
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.
You have
multiplierChanged
onModelAnimation
and I see that that's being used. But this is onModelAnimationCollection
and I don't see that being set or accessed anywhere.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.
Ah, yes, you're right. My bad!