ActionManagerTest CrashTest misleading comments and timing.

[Tosik86]

Hello all!

I continue learning AXMOL and try to understand how engine works by running different tests from cpp-tests project.

Today I was playing around with ActionManagerTests. First test from this suit is CrashTest As far as I understood from the code and name an idea here is to test that AXMOL does not crash if we remove Node while ActionInterval is ongoing.

To be honest I found that comments and also input time arguments are a little bit misleading.
ActionManagerTest.cpp:

    // Sum of all action's duration is 1.5 second.
    child->runAction(RotateBy::create(1.5f, 90));
    child->runAction(Sequence::create(DelayTime::create(1.4f), FadeOut::create(1.1f), nullptr));

    // After 1.5 second, self will be removed.
    child->runAction(Sequence::create(DelayTime::create(1.4f),
                                      CallFunc::create(AX_CALLBACK_0(CrashTest::removeThis, this)), nullptr));

So, this is my understanding of what is going on:

1. We create RotateBy ActionInterval which suppose to rotate our Sprite to 90 degrees during 1,5 seconds.
2. Then we create Sequence of 2 ActionIntervals: 1,4 seconds delay and then FadeOut which suppose to happen during 1,1 seconds after delay.
3. At the same time we create another Sequence also with 2 ActionIntervals: again 1,4 seconds delay and then custom callback removeThis.

Here is a list of problems which I see:

PROBLEM 1
First of all it is not clear what does mean comment: "Sum of all action's duration is 1.5 second". Actions duration are added up only in Sequence, correct? If we call runAction several times, then we create actions which runs "simultaneously". So, what is the point to do "sum of all actions duration"?

PROBLEM 2
It also seems for me that second comment is just not correct: "After 1.5 second, self will be removed". My understanding is that it will be removed after 1.4 seconds, not after 1.5 seconds. I double checked it by adding logging:

This is an output of console:

So, time diff between "onEnter" and "removeThis" is 1420 ms.

PROBLEM 3
Why FadeOut time is 1.1 seconds? What is the point to have this exact value if FadeOut IntervalAction will not have a chance to run in any case.
My proposal of timing would be following:

    child->runAction(RotateBy::create(3.0f, 90));
    child->runAction(Sequence::create(DelayTime::create(1.5f), FadeOut::create(1.5f), nullptr));

    child->runAction(Sequence::create(DelayTime::create(1.5f),
                                      CallFunc::create(AX_CALLBACK_0(CrashTest::removeThis, this)), nullptr));

So, we start rotation which suppose to last for 3 seconds, and after 1,5 seconds also FadeOut should be started and continue simultaneously with rotation. But since we also call another sequence with removeThis callback, then rotation will be interrupted at 1.5 seconds and fading out will never has a chance to start.

Does this sound reasonable?

[Tosik86]

I created PR as proposal of changes: #2156

[Tosik86]

Update:
Hm... actually original code just does not make sense:

It seems that this code is looking for Grossini Sprite by TAGID, but then trying to remove it from its own children list. Of course nothing happen. We should remove it from the CrashTest Scene, as far as I understand. I fixed it in the same PR.

[rh101]

If you're attempting to learn Axmol, then I can only suggest that you avoid any code related to crash testing, since they were written to, quite literally, reproduce a crash. It was not meant to be used for the purpose of learning the features and functionality of the engine.

Whatever caused the crash would have been fixed since then, but that does not mean the logic in those implementations should be changed at all. You don't want to modify the crash test code, because that may be changing whatever it is that triggered the crashes in the first place.