Add unequip animation during stance switching (bug #4327) #1619
Conversation
|
|
||
| float complete; | ||
| bool animPlaying = mAnimation->getInfo(mCurrentWeapon, &complete); | ||
| if (animPlaying && complete < 0.95f) |
There was a problem hiding this comment.
I do not know how to play two animation groups one by one seamlessly, so I have to use a hack - play the equipping animation if more than 95% of unequipping animation was played.
Is there any better solution?
Nevermind, there is another bug in switching itself, will try to investigate it.
akortunov
changed the title
|
Also OpenMW misses unequip sound during weapon->hth switch. That's because we do not know in CharacterController::updateWeaponState() which weapon was equipped previously. UPDATE: added missing sounds for now. |
akortunov
changed the title
akortunov
changed the title
|
I suppose this PR is ready for review/testing now. |
That's an ongoing issue affecting many other animations, and for attack animations in particular we have an open bug. In this PR, it seems that the lack of continuity actually causes a visible glitch rather than just delays/stuttering. The 95% number attempts to prevent this, but will not do so with lower frame rates. Maybe you could set the first animation to not auto-disable so that if there is a frame where the first animation is finished but the second is not started yet, we don't glitch to a position that's related to neither of the two animations. When that's done, the 95% 'hack' could be removed. I just noticed you already made a change to that. I'd still prefer a less hacky one, though. |
Can you tell what's wrong? I'll try to rework it. |
|
I do not like the 'Disable default idle state during weapon [un]equipping to avoid body desync', it looks like a hack and is placed in a totally unrelated section of the code. My suggestion was to set the 'bool autodisable' of playAnimation to false. Not sure if it's practical, but worth a try. |
Done. |
akortunov
changed the title
|
Works well for me. |
|
@zinnschlag can you cast your eye here. :) |
| 1.0f, "unequip start", "unequip stop", 0.0f, 0); | ||
| mUpperBodyState = UpperCharState_UnEquipingWeap; | ||
|
|
||
| if(!downSoundId.empty() && !isStillWeapon) |
There was a problem hiding this comment.
!isStillWeapon is already checked above this.
| { | ||
| if(stats.getDrawState() == DrawState_Spell) | ||
| if(!getRealWeapon && stats.getDrawState() == DrawState_Spell) |
There was a problem hiding this comment.
The 'getRealWeapon' parameter seems a bit messy, is there any way to refactor the code so we don't need it?
| @@ -152,6 +152,7 @@ struct WeaponInfo; | |||
| class CharacterController : public MWRender::Animation::TextKeyListener | |||
| { | |||
| MWWorld::Ptr mPtr; | |||
| MWWorld::Ptr mPreviousWeapon; | |||
There was a problem hiding this comment.
I think mWeapon would be a better name because this is the up to date weapon more often than it isn't.
|
I uploaded reworked version (changes, requested by scrawl). An additional question: should we use weapon unequipping sound during weapon->spell switch? |
akortunov
changed the title
akortunov
changed the title
|
Are there any complaints or suggestions? |
Fixes bug #4327 and bug #2835.
Since CharacterController::updateWeaponState() has more than 500 lines of code, I am not able to test all possible cases for my own.
Also I noticed that there is no unequipping sound in Morrowind during weapon->spell switching. Should I add it?
Note: I am not sure if I should show torches and/or shields during stance change.