Skip to content
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

Walk/run animation blending #1947

Merged
merged 10 commits into from
Dec 12, 2021
Merged

Walk/run animation blending #1947

merged 10 commits into from
Dec 12, 2021

Conversation

skaljac
Copy link
Contributor

@skaljac skaljac commented Dec 7, 2021

Issue related to this PR:
#1695

Steps for testing:
Walk around
Press Shift to start running
Switch from walk to run and vice versa should not be instant

Verified

This commit was signed with the committer’s verified signature.
fiji-flo Florian Dieminger
@skaljac
Copy link
Contributor Author

skaljac commented Dec 7, 2021

2021-12-06.18-31-24.mp4

Copy link
Contributor

@torchesburn torchesburn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me.

Will test

@alisaad673
Copy link
Contributor

Device:
Dell precision 7710
Ram: 16gb
Ci7 6th gen
Nvidia Quadro m5000m (60Hz refresh rate)

Issues:

1-Walk to run is smooth now but run to walk appears quite instantaneous and a bit jerky.

run.walk.intant.mp4

2-Walk to run is smooth as whole but while closely looking at footsteps while the blend happens, The footsteps appear a bit off and different than original flow. They appear to be out of synchronization for a moment.

stepstart.footstepissue.mp4

3-While we start with run(By pressing shift+W) instead of first walking and then shifting to run, There appears an instant jerk at the start.

starting.with.run.mp4

4-At low fps of around 25-40, The avatar slows down while running. Sometimes it doesn't run at low fps.

run.slow.speed.mp4
runatlowfps.mp4

@@ -1635,6 +1643,9 @@ class Avatar {
const currentSpeed = localVector.set(this.velocity.x, 0, this.velocity.z).length();
const angle = this.getAngle();
const timeSeconds = now/1000;

this.speedValue = lerpFloat(this.speedValue,currentSpeed,0.1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks dependent on the frame time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have use fimeDiff in this calculation


//console.log(currentSpeed);
// Blend: idle <-> walk
if (currentSpeed <= 0.45) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right way to do it. We can smoothly blend on a curve over the
instantaneous velocity, not an if statement plus a lerp to switch between walk/run modally.

Verified

This commit was signed with the committer’s verified signature.
caugner Claas Augner
@skaljac
Copy link
Contributor Author

skaljac commented Dec 8, 2021

I have resolve main issue with this code. It is not modal. It is smooth.

Works as expected.

Also I had to introduce float speedValue (it is lerping to speed value, it respects timeDiff) beacuse there is no gradual change in speed between walk and run.

@skaljac
Copy link
Contributor Author

skaljac commented Dec 8, 2021

2021-12-08.23-57-06.mp4

@alisaad673
Copy link
Contributor

alisaad673 commented Dec 9, 2021

1-Crouch run can be improved as The transition from standing to walk and vice versa seems a bit fast and jerky

crouchrun.mp4

2-The footsteps at the start of walk/run transition seem a bit off and needs improvement.

footstepatstartissue.mp4

The purpose of this PR is somewhat fulfilled. The walk/run transition has improved, the above mentioned issues can be solved in later PR's. So I recommend merging the current improvement into master.

@avaer
Copy link
Contributor

avaer commented Dec 12, 2021

This needs a rebase after merging #1856

@skaljac
Copy link
Contributor Author

skaljac commented Dec 12, 2021

Rebase is done

@avaer
Copy link
Contributor

avaer commented Dec 12, 2021

Made some major timing fixes, blending should be smooth now.

@avaer avaer merged commit 24c68d9 into master Dec 12, 2021
@skaljac skaljac self-assigned this Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants