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

Reset movement inside leap to show attack animation #16813

Merged
merged 1 commit into from Aug 17, 2019

Conversation

@Hedog
Copy link
Contributor

commented Jul 20, 2019

Fixes #16747

The dog eat animation will not trigger because in the WithInfantryBody.cs Tick function the code has changed in commit a10af38 (WithInfantryBody.cs Line 143)

- if ((state != AnimationState.Moving || dirty) && move.IsMoving)
+ if ((state != AnimationState.Moving || dirty) && move.CurrentMovementTypes.HasFlag(MovementType.Horizontal))

Before the change the move animation would only trigger when move.IsMoving is explicitely set to true. Inside Leap.cs move.IsMoving was set to false, when the leaper reached its target, so the move animation will not run.

With the new code the move animation will run instead of the attack animation because the currentMovementTypes has the flag MovementType.Horizontal which will be set in the Mobile.cs Tick function. The Mobile.cs Tick function will count the leap as horizontal movement which is technically correct but in this case we want to directly set the leaper to its target position without setting the movement type.

To prevent the Mobile.cs Tick function to register a horizontal movement I set the oldPosition to the CenterPosition so it won't detect any movement which is what we want here.

@abcdefg30 abcdefg30 added this to the Next Release milestone Jul 20, 2019

@pchote

This comment has been minimized.

Copy link
Member

commented Jul 27, 2019

Sorry for the radio silence here so far... It seems that the issue with CurrentMovementTypes updating too late is causing problems with the new pathfinding logic, and possibly other things too.

@teinarss and @reaperrr are investigating this, but I suspect that your approach here is likely the way forward. If it is we will need to do some tweaks to generalize it to cover all the things that can move the actor.

@pchote

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

#16861 ended up not needing to touch CurrentMovementTypes, so we can proceed here.

Rather than introducing a new Reset method, it would be cleaner if we extracted the contents of Mobile's Tick into a public void UpdateMovementTypes(Actor self) method which is called by both Tick and Leap.

This avoids introducing a special case workaround, and means that any places that need to immediately resolve the CurrentMovementTypes in the other direction (stationary → moving) can call the same method.

@Hedog Hedog force-pushed the Hedog:dog-eat-animation branch from b9cb1d4 to e1dd73a Aug 12, 2019

@pchote
pchote approved these changes Aug 16, 2019
@pchote

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

GitHub doesn't send notifications when a PR is updated, so its generally a good idea to leave a comment so we know to come back to it 😄

@abcdefg30 abcdefg30 merged commit 2a6f2bb into OpenRA:bleed Aug 17, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.