Skip to content

Conversation

@stephengold
Copy link
Member

This solves a bug seen in TestJaime where anim events in the Cinematic were occasionally suppressed. This happened when 2 events were scheduled to play consecutively on the same layer and the later event's onStart() was invoked before the earlier event's onStop(). In this situation onStop() would remove the wrong action from the layer.

@stephengold stephengold added the bug Something that is supposed to work, but doesn't. More severe than a "defect". label May 4, 2021
@stephengold stephengold added this to the v3.4.0 milestone May 4, 2021
@capdevon
Copy link
Contributor

capdevon commented May 5, 2021

Hi Stephen,
perhaps it is better to use the animComposer.getAction(actionName) method, instead of the animComposer.action(actionName) method which is also used to create actions and could be confusing.

as-is

Action currentAction = composer.getCurrentAction(layerName);
Action eventAction = composer.action(actionName);
if (currentAction == eventAction) {
	composer.removeCurrentAction(layerName);
}

to-be

Action currentAction = composer.getCurrentAction(layerName);
Action eventAction = composer.getAction(actionName);
if (currentAction == eventAction) {
	composer.removeCurrentAction(layerName);
}

@stephengold
Copy link
Member Author

Thanks for the suggestion.

@stephengold stephengold marked this pull request as draft May 7, 2021 17:27
@stephengold
Copy link
Member Author

I've decided a better fix is needed, to correctly handle the situation where the 2 consecutive events both play the same Action.

@stephengold stephengold marked this pull request as ready for review May 7, 2021 18:55
@stephengold stephengold merged commit cf0fae6 into master May 8, 2021
@stephengold stephengold deleted the sgold-fix-animevent branch May 8, 2021 15:44
stephengold added a commit that referenced this pull request May 8, 2021
…er (#1537)

* AnimEvent: solve bug where onStop() kills prior event on the same layer

* AnimEvent: for clarity, use getAction() instead of action()

* AnimComposer: keep track of the manager for each layer

* AnimEvent: keep track of which event is managing each layer

* TestJaime: reset the model position each time the cinematic starts
@stephengold
Copy link
Member Author

I confirmed that the issue is resolved in v3.4.0-beta3

Mezage pushed a commit to Mezage/jmonkeyengine that referenced this pull request May 27, 2021
@Mezage Mezage mentioned this pull request May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something that is supposed to work, but doesn't. More severe than a "defect".

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants