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

Fix: Sil/Sword combat and aim animation problem while walking/running. #2791

Closed
wants to merge 36 commits into from

Conversation

gonnavis
Copy link
Contributor

@gonnavis gonnavis commented Apr 8, 2022

Fix: #2049

Preliminarily fixed this issue. The main changes are these lines.

Basic idea is to not apply sword combo animations to legs when walking.

Now not very awkward, but still has many todos:

  • Lerp combo and walk/run based on moveFactors.idleWalkFactor.
  • Check bones by whether shoulder and arm instead of isTop.
  • Fix when crouch.
  • Test with silsword.
sword.walk.mp4

@avaer
Copy link
Contributor

avaer commented Apr 8, 2022

Looks good! Let's also make sure we are blending correctly for aim mode too (right click mouse).

@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 9, 2022

Crouch:

combo.3.crouch.mp4

Aim:

combo.4.lock.mp4

Hello @avaer , is it ok?

There's still a lot of room for optimization, and with silsword, still looks a little awkward.
But the most obvious ones should be addressed for now, maybe keep fixing in other PRs.
image

@gonnavis gonnavis marked this pull request as ready for review April 9, 2022 09:53
@gonnavis gonnavis requested a review from avaer April 9, 2022 09:54
@gonnavis gonnavis changed the title Fix: Sword combat animation while walking/running. Fix: Sword combat and SilSword aim animation while walking/running. Apr 9, 2022
@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 9, 2022

Noticed that silsword's aim is different from sword's aim logic/codes.
So fixed it's animation problem while walk/run too.

aim.1.mp4

@gonnavis gonnavis changed the title Fix: Sword combat and SilSword aim animation while walking/running. Fix: Sil/Sword combat and aim animation problem while walking/running. Apr 9, 2022
@avaer
Copy link
Contributor

avaer commented Apr 11, 2022

Noticed that silsword's aim is different from sword's aim logic/codes.

Yes, the animations are not related at all.

Re: the above, I think the walk/run needs to be blended into the arms when aiming the sword or else it will look wrong/stiff.

@gonnavis
Copy link
Contributor Author

Blended walk/run into arms when aiming.

aim.2.blend.arm_1.mp4

@gonnavis
Copy link
Contributor Author

There is an animation for aiming the sword, but it is much too fast.

Tested that 300ms too slow, so increased to 200ms. 8dfa702

@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 15, 2022

There is no animation for unaiming the sword.

Also fix: #2573

Generally handled end-action transition, works well for jumping-end, unaiming and attacking-end. 441b049

general.end.action.transition.2.mp4

};
} else if (activeAvatar.unuseAnimation && activeAvatar.unuseTime >= 0) {
return spec => {
return (spec, isLastBone) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm this isn't a good approach. If something should happen after the last bone then the caller should do that, not the quaternion handler.

Copy link
Contributor

Choose a reason for hiding this comment

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

This might actually affect performance negatively to pass extra arguments to all bones for all frames.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, @avaer , is this proper? dd0ea6f

@avaer
Copy link
Contributor

avaer commented Apr 15, 2022

Hm I don't know if the jump landing is better. It seems to take too long for some cases.

Let's not mix this up with the sword PR. We can do actual landing animations which will look much better.

Also not related to this PR: the spine sword problem is more obvious when you try to attack while running.

@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 15, 2022

Hm I don't know if the jump landing is better. It seems to take too long for some cases.
Let's not mix this up with the sword PR. We can do actual landing animations which will look much better.

Reverted generally handle end-action transition 441b049 .
I'll trying in subsequent PR.

@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 15, 2022

the spine sword problem is more obvious when you try to attack while running.

Ok, I think I should add back isCombo bone group which includes arms, Chest and UpperChest.
Otherwise the attack range will reduced a lot by the absent of Chest and UpperChest while walking/running.
What's the name is better, isUpperBody, isAttack, isSword?

And I think we need dedicated attack animations for walking/running/crouching eventually, because these case are way too different from standing attack, can't make good by just blending standing attack.

And maybe we can prevent walking/running/crouching attacking totally, like many other action games do.
If want attack, will auto-stop walking/running/crouching ( temporarily ).
Then this PR is mainly fixing aiming ( or "aggro" state ) animation while walking/running/crouching.

@gonnavis gonnavis requested a review from avaer April 17, 2022 09:18
@@ -35,6 +35,16 @@ const {CharsetEncoder} = require('three/examples/js/libs/mmdparser.js');
'Crouched Sneaking Left.fbx',
'Crouched Sneaking Right.fbx',
];
const trimClip = (clip, startTime, endTime) => {
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 ok to ship, but I think we can fix this in the animation itself in the long term.

Maybe add a comment here/open an issue to trim this animation in Blender.

Copy link
Contributor Author

@gonnavis gonnavis Apr 18, 2022

Choose a reason for hiding this comment

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

Ok, added comment first. eb1b8da
I also want to trim this sword animation into three pieces, if the other pr can be merged #2804 .
I'll request artist asset afterwards.

@gonnavis
Copy link
Contributor Author

Resolved conflicts.

@gonnavis
Copy link
Contributor Author

gonnavis commented Apr 19, 2022

Reverted fixing of unuseAnimation not stop problem, 4dc4625
because it's not very related to this PR and not a serious problem,
should try in another PR afterwards.

@avaer
Copy link
Contributor

avaer commented Apr 21, 2022

After testing again:

I still think this PR is far too robotic when walking and attacking. I think overall it is not better than what we had before, it's just a different set of animation problems we are trading for.

At least what we had before was very dynamic even if all of the bones were going crazy.

If we need walk-attack and run-attack custom animations to solve this and make powerful swipes that do not look like a robot (especially in the spine), I think that is a valid plan.

@gonnavis
Copy link
Contributor Author

Got it! I'll contact artist and request resource.

@gonnavis gonnavis marked this pull request as draft April 21, 2022 08:45
@gonnavis
Copy link
Contributor Author

gonnavis commented May 2, 2022

Only improve silsword with aimAnimation first, reverted changes of useAnimations.
I feel it's better than before, how about only improve sword aimAnimation first?

Before:

aim.sword.before.mp4

After:

aim.sword.after.mp4

@gonnavis gonnavis marked this pull request as ready for review May 2, 2022 14:01
@gonnavis gonnavis marked this pull request as draft May 13, 2022 19:24
@avaer avaer closed this Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

app: Sword combat animation while walking/running
2 participants