Paths

Table of Contentst

BGE: fix animation waking up for suspended scenes.
AbandonedPublic
Actions

Authored by Porteries Tristan (panzergame) on Oct 20 2015, 7:53 PM.

Details

Reviewers

Ulysse Martin (youle)
Jorge Bernal (lordloki)
Mitchell Stokes (moguri)

Summary

rBSd979ac4cc9967c5dce27461926396abe5de54b3d introduced artefact with animations on suspended scenes.
The problem come form the suspended delta time (scene pause time) set in the main loop.
In the case of 2 scenes, if the first is suspended, not proceed and that the second resume it, the pause time will be not set for the first scene. The cause of wrong pause time create in BL_Action::Update the artifcats, indeed the action do localtime = curtime - pausetime to manage properly pauses but in this case pausetime = 0 and for the action there was no scene's pause. It's like the action was played during the scene's pause.
To fix it the patch just set pause time properly by explode the main loop, this allow to get the proper scene status (suspended/resumed) after the main loop and set the pause time cooresponding.

Example file :

D1569_animation_suspended_scene.blend490 KBDownload

: press S to suspend and R to resume.

Diff Detail

Repository: rB Blender
Branch: ge_animation_suspended_scene

Event Timeline

Porteries Tristan (panzergame) updated this revision to Diff 5250.Oct 20 2015, 7:53 PM

Porteries Tristan (panzergame) retitled this revision from to BGE: fix animation waking up for suspended scenes..

Porteries Tristan (panzergame) updated this object.

Porteries Tristan (panzergame) added reviewers: Mitchell Stokes (moguri), Sybren A. Stüvel (sybren), Jorge Bernal (lordloki), Ulysse Martin (youle).

Porteries Tristan (panzergame) updated this object.Oct 20 2015, 7:55 PM

At first I found a bit strange to have a scene loop right after the main scene loop, but if it works maybe it's fine. I think I get the problem (a unsuspended scene would affect another scene and that scene would run in this loop still). But I wonder if you could achieve this in the main scene loop,

I would keep the main explanation of the patch in the log though (instead of in-code comments only). It is better for records sakes (git log, git blame, ...).

And: "recalcutlate" > "recalculate"

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
1069	I think it's better if you leave cleanups outside of bugfix commits

At first I found a bit strange to have a scene loop right after the main scene loop, but if it works maybe it's fine. I think I get the problem (a unsuspended scene would affect another scene and that scene would run in this loop still). But I wonder if you could achieve this in the main scene loop,

I can't merge this with the main loop, because a unproceed scene in the main loop don't set its suspended delta time.

Before:

Scene1 : status = Suspended, delta time = 0
Scene2 : status = Activated, delta time = 0

Main loop :
   Scene1 : do nothing
   Scene2 : resume Scene1

Scene1 : status = Activated, delta time = 0
Scene2 : status = Activated, delta time = 0

Render loop:
   Scene1: update animations + render // suspend delta time not set.
   Scene2: update animations + render

Now:

Scene1 : status = Suspended, delta time = 0
Scene2 : status = Activated, delta time = 0

Main loop :
   Scene1 : do nothing
   Scene2 : resume Scene1

Scene1 : status = Activated, delta time = 0
Scene2 : status = Activated, delta time = 0

Suspend loop:
   Scene1 : set delta time
   Scene2 : do nothing.

Scene1 : status = Activated, delta time = 10(eg)
Scene2 : status = Activated, delta time = 0

Render loop:
   Scene1: update animations + render
   Scene2: update animations + render

Porteries Tristan (panzergame) mentioned this in rBf10db730bc4d: BGE: Fix T45945: Action bouncing..Oct 28 2015, 3:11 PM

Porteries Tristan (panzergame) updated this object.Oct 28 2015, 4:15 PM

The code works, but I do have some remarks about it. See inline comments.

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
710	This method is already far too long. Please move this block to a separate method, and call it from here.
733	This is only valid if all scenes that are suspended/resumed are suspended/resumed at exactly the same time. Is this enforced anywhere?

This revision now requires changes to proceed.Nov 1 2015, 10:53 AM

Porteries Tristan (panzergame) added inline comments.Nov 1 2015, 11:03 AM

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
710	What do you think about : UpdateSuspendedScenes() ?
733	Why ? m_suspendedtime and m_suspendeddelta are useless in this block, will test and remove..

Sybren A. Stüvel (sybren) added inline comments.Nov 1 2015, 11:17 AM

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
733	The value of `m_suspendeddelta` is overwritten when there are more than one scene that was just resumed, so after the for-loop it contains `scene->getSuspendedDelta()` from the last scene iterated over. This means that the result is unpredictable, unless all scenes return the same `getSuspendedDelta()`. Unpredictable code is buggy code, so this should be solved.

Porteries Tristan (panzergame) added inline comments.Nov 1 2015, 11:36 AM

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
733	m_suspendeddeltatime is reset in UpdateAnimations, which make the set here useless.

Move scene pause time management in a new function : UpdateSuspendedScenes

Sybren A. Stüvel (sybren) added inline comments.Nov 1 2015, 5:45 PM

source/gameengine/Ketsji/KX_KetsjiEngine.h
250	This needs an explanation of what it does. Right now the comment is in the source file inside the method, so it won't be found by doxygen or other tooling.

Move comment for doxygen

Move UpdateSuspendedScenes under private.
Update comments : replace "scene's suspend delta time" by "scene's total pause time"

Mitchell Stokes (moguri) added inline comments.Nov 13 2015, 11:05 PM

source/gameengine/Ketsji/KX_KetsjiEngine.cpp
1055	Where does m_suspendeddelta get used? Could we remove m_suspendeddelta from KX_KetsjiEngine and just get the value from the scene? If this is a per-scene value, storing it in what is effectively a global variable seems to just be asking for trouble.

Resigning as reviewer, as I don't have time for BGE work any more.

BGE was removed in Blender 2.80 this patch is being closed as a result.

Revision Contents
Changeset List

Path

Size

source/

gameengine/

Ketsji/

KX_KetsjiEngine.h

4 lines

KX_KetsjiEngine.cpp

46 lines

Diff 5357

View Options

source/gameengine/Ketsji/KX_KetsjiEngine.h