Count run action finish to call final finish when two run actions are…

[AlexandreK38]

Describe your changes

Hi,
After migrating from Cocos2d, I discovered that transition animations were not working properly, like TransitionSlideInB for example. The slide made was going more than the max it should.
After digging into this and compare with Cocos2d; I found out that the the inScene/outScene run actions were done in the opposite order leading to override the position.

When there is 2 run actions performed in parallel, the order should not matter (maybe that why there is a warning about issues at top of the files by Cocos2d authors)
This is my proposal to fix this issue: call the finish only when both actions are done.

[smilediver]

Instead of hardcoding _finishedRunActionCount == 2 and having two finishing methods, I think a better design would be to:

• Remove addFinish() to leave a single finish().
• Add int _waitForFinishCount = 1; field and setWaitForFinishCount(int count).
• Before running the actions call setWaitForFinishCount(2) if required.
• Decrement _waitForFinishCount in finish() until it reaches 0 to trigger the finish logic.

[IamSanjid]

both in scene and out scene actions are done on the same single thread right? but not in sequential order? or different idle threads might pick up and execute those actions?

if they're on the same thread then i think it should be called sequentially. so, may be just a simple state variable which keeps track of which last action was finished before finishing current action, would be enough. otherwise, if multiple threads might run those in parallel we should make this _finishedRunActionCount an atomic variable too

[rh101]

both in scene and out scene actions are done on the same single thread right? but not in sequential order? or different idle threads might pick up and execute those actions?

Scene transitions simply use actions, and they run on the main thread, so no multi-threading involved.

[IamSanjid]

both in scene and out scene actions are done on the same single thread right? but not in sequential order? or different idle threads might pick up and execute those actions?

Scene transitions simply use actions, and they run on the main thread, so no multi-threading involved.

So they should be executed sequentially right?

[AlexandreK38]

both in scene and out scene actions are done on the same single thread right? but not in sequential order? or different idle threads might pick up and execute those actions?

Scene transitions simply use actions, and they run on the main thread, so no multi-threading involved.

So they should be executed sequentially right?

I’m not sure how the transitions are stacked and processed but somehow the outScene one was done before the inScene one, leading to position override.
It’s strange no one saw this issue before 🤔

[AlexandreK38]

Instead of hardcoding _finishedRunActionCount == 2 and having two finishing methods, I think a better design would be to:

• Remove addFinish() to leave a single finish().
• Add int _waitForFinishCount = 1; field and setWaitForFinishCount(int count).
• Before running the actions call setWaitForFinishCount(2) if required.
• Decrement _waitForFinishCount in finish() until it reaches 0 to trigger the finish logic.

I thought about adding a method like setWaitForFinishCount(int count) but in fact 2 is also hard coded. In fact we will always call your method with 2 as parameters, so maybe setWaitForTwoFinish() or similar and set _waitForFinishCount = 2; in

[IamSanjid]

I’m not sure how the transitions are stacked and processed but somehow the outScene one was done before the inScene one, leading to position override. It’s strange no one saw this issue before 🤔

hmm weird those actions are stored in ax::Vector which uses std::vector under the hood, in scene and out scene both should be part of the same node too, weird can u provide some simplified example how you're using the Transition*(s)?

[AlexandreK38]

I’m not sure how the transitions are stacked and processed but somehow the outScene one was done before the inScene one, leading to position override. It’s strange no one saw this issue before 🤔

hmm weird those actions are stored in ax::Vector which uses std::vector under the hood, in scene and out scene both should be part of the same node too, weird can u provide some simplified example how you're using the Transition*(s)?

Yeah it’s strange

I’m not sure how the transitions are stacked and processed but somehow the outScene one was done before the inScene one, leading to position override. It’s strange no one saw this issue before 🤔

hmm weird those actions are stored in ax::Vector which uses std::vector under the hood, in scene and out scene both should be part of the same node too, weird can u provide some simplified example how you're using the Transition*(s)?

Yeah the finish was called by the outScene action because it was supposed to be the last of the 2 actions called (for all transitions kind) but it’s not the case anymore compared to Cocos2dx; afaik there is no reason one run will end before the other

my code is quite simple, being in the MainScene, I just create a GameScene (subclass of ax::Scene) and pushing it using a TransitionSlideInB

GameScene *scene = GameScene::create();
scene->loadLevel(last);
Director::getInstance()->pushScene(ax::TransitionSlideInB::create(0.3f, scene));

[smilediver]

I thought about adding a method like setWaitForFinishCount(int count) but in fact 2 is also hard coded. In fact we will always call your method with 2 as parameters, so maybe setWaitForTwoFinish() or similar and set _waitForFinishCount = 2; in

It would be not hardcoded, but parameterized, generalizing the logic and allowing users to specify the finish() call count. With addFinish() and finish() you're splitting the logic into two separate paths, while setWaitForFinishCount() keeps the logic same for all cases.

[rh101]

So they should be executed sequentially right?

I think I understand where you're coming from, but that's not quite how it works. As mentioned earlier, scene transitions use actions. Multiple actions can run at the same time, in parallel, but not in the sense of being parallel threaded, but rather in that they are executing within the same update loop.

In the case of scene transitions, whatever operation is being applied to the in and out scenes are happening in parallel, and the visit() method is called on both scenes to render them to the screen.

If you want to learn more about how actions work, then browse through the Action.cpp/ActionInterval.cpp files etc..

[rh101]

When there is 2 run actions performed in parallel, the order should not matter (maybe that why there is a warning about issues at top of the files by Cocos2d authors)

What warning are you referring to?

Also, do you have a video or sample project to clearly show the issue? I'm not quite sure I understand what you meant by "The slide made was going more than the max it should." for TransitionSlideInB.

The actions applied on the scenes end at a specific duration, and since the order of completion is causing an issue, a possible alternative fix would bet to add a DelayTime action of 0 duration to the action sequence that is responsible for calling the finish() method. In the case of TransitionSlideInB, it would force the out scene to finish on the next update, after both the in scene and out scene primary actions are complete. This is assuming both primary actions applied to the in and out scenes are exactly the same duration (which should be the case on all such transitions).

For example, this is the implementation for all slide transitions:

ActionInterval* in  = this->action();
ActionInterval* out = this->action();

ActionInterval* inAction  = easeActionWithAction(in);
ActionInterval* outAction = Sequence::create(
    easeActionWithAction(out), CallFunc::create(AX_CALLBACK_0(TransitionScene::finish, this)), nullptr);
_inScene->runAction(inAction);
_outScene->runAction(outAction);

Try changing this:

ActionInterval* outAction = Sequence::create(
    easeActionWithAction(out), CallFunc::create(AX_CALLBACK_0(TransitionScene::finish, this)), nullptr);

to this:

ActionInterval* outAction = Sequence::create(
    easeActionWithAction(out), DelayTime::create(0), 
    CallFunc::create(AX_CALLBACK_0(TransitionScene::finish, this)), nullptr);

[rh101]

Just FYI, the reason this scene transition bug has appeared in Axmol is because of a few reasons:

1 - ActionManager was updated in Axmol to use a std::unordered_map to store the nodes, so now the order is based on the hash value, and not based on the insertion order (which I assume is how it was working previously). You can see the change here: c2847f8
2 - The scene transition implementations were never correct to begin with, and the only reason they worked in Cocos2d-x was purely by chance. When running actions on two separate nodes, there is no way to sync those actions, and if the final result is dependent on unrelated node action sequences, then there needs to be some mechanism in place to detect that, which is what this PR is addressing.

[AlexandreK38]

What warning are you referring to?

This comment in Transaction.cpp, about the "other issue". I think they faced what we encountered but never discovered why.

//
// SlideInL
//

// The adjust factor is needed to prevent issue #442
// One solution is to use DONT_RENDER_IN_SUBPIXELS images, but NO
// The other issue is that in some transitions (and I don't know why)
// the order should be reversed (In in top of Out or vice-versa).

The actions applied on the scenes end at a specific duration, and since the order of completion is causing an issue, a possible alternative fix would bet to add a DelayTime action of 0 duration to the action sequence that is responsible for calling the finish() method. In the case of TransitionSlideInB, it would force the out scene to finish on the next update, after both the in scene and out scene primary actions are complete. This is assuming both primary actions applied to the in and out scenes are exactly the same duration (which should be the case on all such transitions).

Adding a 0 delay is not the best to me as it would mean -as you said- that it would trigger at next update, so the transition delay would not be exactly what the user asked for.

Just FYI, the reason this scene transition bug has appeared in Axmol is because of a few reasons:

1 - ActionManager was updated in Axmol to use a std::unordered_map to store the nodes, so now the order is based on the hash value, and not based on the insertion order (which I assume is how it was working previously). You can see the change here: c2847f8 2 - The scene transition implementations were never correct to begin with, and the only reason they worked in Cocos2d-x was purely by chance. When running actions on two separate nodes, there is no way to sync those actions, and if the final result is dependent on unrelated node action sequences, then there needs to be some mechanism in place to detect that, which is what this PR is addressing.

Yeah it sounded like that when I looked at how it was done, the issue was already there but it worked by chance. The unordered map explains the order, thanks for info!

BTW i updated the PR to have the setWaitForFinishCount()

[IamSanjid]

Just FYI, the reason this scene transition bug has appeared in Axmol is because of a few reasons:

1 - ActionManager was updated in Axmol to use a std::unordered_map to store the nodes, so now the order is based on the hash value, and not based on the insertion order (which I assume is how it was working previously). You can see the change here: c2847f8 2 - The scene transition implementations were never correct to begin with, and the only reason they worked in Cocos2d-x was purely by chance. When running actions on two separate nodes, there is no way to sync those actions, and if the final result is dependent on unrelated node action sequences, then there needs to be some mechanism in place to detect that, which is what this PR is addressing.

ah right now makes sense why for some reason i thought the in scene and out scene would be part of the same node.
may be we somehow tell the ActionManager to treat them as part of the same node?

[rh101]

may be we somehow tell the ActionManager to treat them as part of the same node?

That is not possible, and it wouldn't make sense since the actions are in fact not actually operating on the same node. This really has nothing to do with the ActionManager.

A more useful element would be to add a new object something like ParallelActions which would be an action running several actions (which could be sequences) and then run a final action (which can be a callback) when the previous actions are all done

From what I have seen in the action system, it is not easily adaptable to support this. If you go through the different types of actions, especially ones inheriting from FiniteTimeAction, and how action sequences actually work, then you will see why.

[IamSanjid]

may be we somehow tell the ActionManager to treat them as part of the same node?

That is not possible, and it wouldn't make sense since the actions are in fact not actually operating on the same node. This really has nothing to do with the ActionManager.

hmm so i guess the whole Transition system should really introduce some sort of state system, it might be even make life easy to implement new sequence actions dependant Transition types, and it makes sense to have each Transition to save their current state too.

[smilediver]

hmm so i guess the whole Transition system should really introduce some sort of state system

That's exactly what _waitForFinishCount is for. I don't think any more complicated systems are necessary, and they would just complicate things instead of solving real problems.

That being said, if there was WaitForAction action which waits for another action's completion, then transitions could be refactored to use that instead of _waitForFinishCount. That would also open up some other possibilities.

[IamSanjid]

That's exactly what _waitForFinishCount is for. I don't think any more complicated systems are necessary, and they would just complicate things instead of solving real problems.

That being said, if there was WaitForAction action which waits for another action's completion, then transitions could be refactored to use that instead of _waitForFinishCount. That would also open up some other possibilities.

I guess it just makes sense for current situation, and waiting for 2 actions kinda feels arbitary, doesn't feel like a general solution, it doesn't answer what type of action we're waiting for to be finished, what if some other unexpected actions finishes after calling setWaitForFinishCount(2) (even tho nothing else seems to be calling Tansition::finish). so, if we just want to fix the "out scene" getting finished first before "in scene" situation then we could just hard code it some other way without introducing functions like setWaitForFinishCount, I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

[thomascastel]

That's exactly what _waitForFinishCount is for. I don't think any more complicated systems are necessary, and they would just complicate things instead of solving real problems.
That being said, if there was WaitForAction action which waits for another action's completion, then transitions could be refactored to use that instead of _waitForFinishCount. That would also open up some other possibilities.

I guess it just makes sense for current situation, and waiting for 2 actions kinda feels arbitary, doesn't feel like a general solution, it doesn't answer what type of action we're waiting for to be finished, what if some other unexpected actions finishes after calling setWaitForFinishCount(2) (even tho nothing else seems to be calling Tansition::finish). so, if we just want to fix the "out scene" getting finished first before "in scene" situation then we could just hard code it some other way without introducing functions like setWaitForFinishCount, I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

It is not arbitrary: each transition can override the value to set up the necessary one

[thomascastel]

That's exactly what _waitForFinishCount is for. I don't think any more complicated systems are necessary, and they would just complicate things instead of solving real problems.
That being said, if there was WaitForAction action which waits for another action's completion, then transitions could be refactored to use that instead of _waitForFinishCount. That would also open up some other possibilities.

I guess it just makes sense for current situation, and waiting for 2 actions kinda feels arbitary, doesn't feel like a general solution, it doesn't answer what type of action we're waiting for to be finished, what if some other unexpected actions finishes after calling setWaitForFinishCount(2) (even tho nothing else seems to be calling Tansition::finish). so, if we just want to fix the "out scene" getting finished first before "in scene" situation then we could just hard code it some other way without introducing functions like setWaitForFinishCount, I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

Another solution would be to create a special CallFunc Action that will be added to both sequences and that will manage the count

[smilediver]

I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

I agree. I just disagreed that the solution should be to complicate the transition system itself by adding specialized synchronization system to it. So my proposal was to extend the existing action system by introducing an action that can wait for other action completion. This solution would allow to synchronize actions on different nodes.

Another solution would be to create a special CallFunc Action that will be added to both sequences and that will manage the count

I think WaitForAction would be more flexible, because it's an action and composes easily, something like:

Sequence::create(
    SomeAction::create(),
    WaitForAction::create(actionOnAnotherNode1, actionOnAnotherNode2, ...);
    SomeOtherAction::create()
);

With a CallFunc like thing you'd have to do this inside its callback. Also it would require inserting something into the action list and it would not work in situations when you can only get a pointer to an action like calling some Action* getSomeAction() method.

[IamSanjid]

Another solution would be to create a special CallFunc Action that will be added to both sequences and that will manage the count

I believe the current solution provided by @AlexandreK38 does that, it calls the "special" version of the finish function at the end of both in and out scene actions to maintain the "count"

It is not arbitrary: each transition can override the value to set up the necessary one

The 2 is just a lucky number here, that only two node(s) will call the finisth function which maintains the count by chance, this is probably alright for most of the Transition but assuming that only 2 or 3 number of actions will run is not a good solution, at least you should be able to detect or choose which actions were ran

[IamSanjid]

I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

I agree. I just disagreed that the solution should be to complicate the transition system itself by adding specialized synchronization system to it. So my proposal was to extend the existing action system by introducing an action that can wait for other action completion. This solution would allow to synchronize actions on different nodes.

Another solution would be to create a special CallFunc Action that will be added to both sequences and that will manage the count

I think WaitForAction would be more flexible, because it's an action and composes easily, something like:

Sequence::create(
    SomeAction::create(),
    WaitForAction::create(actionOnAnotherNode1, actionOnAnotherNode2, ...);
    SomeOtherAction::create()
);

With a CallFunc like thing you'd have to do this inside its callback. Also it would require inserting something into the action list and it would not work in situations when you can only get a pointer to an action like calling some Action* getSomeAction() method.

Ye that will be also be a good solution overall and by adding some state to Transition i meant usually when you think about Transition, it should have an update function which should first check if State1(may be moving to x, y position) is finished if not do that task and save State1 finished then continue with State2(which may be decreases opacity over time) if State1 is finished. I didn't check deep enough to fully understand on how axmol/cocos2dx manges this kind state of each transition, but it appears they kind of don't and instead these states are converted into actions and the ActionManager kind of works as the state manager. But I think this action approach kind of makes easier to implement different kinds of Transition without writing huge if/else/switch block, but then again it suffers from some overhead execution and the syncronization issue too.

[rh101]

I guess it just makes sense for current situation, and waiting for 2 actions kinda feels arbitary, doesn't feel like a general solution, it doesn't answer what type of action we're waiting for to be finished, what if some other unexpected actions finishes after calling setWaitForFinishCount(2) (even tho nothing else seems to be calling Tansition::finish). so, if we just want to fix the "out scene" getting finished first before "in scene" situation then we could just hard code it some other way without introducing functions like setWaitForFinishCount, I am just saying waiting for 2 tasks/actions kinda really feels hard coded solution/solution of a specific problem, but yet it's trying to be a general type of solution.

Just to clarify a few things. Firstly, there is nothing arbitrary about waiting for 2 unconnected actions (or action sequences) to finish, because in the case of scene transitions, there are only 2 scenes that each have their own actions applied. There can be no more than 2 scenes. These scenes are not connected in any way, and neither are there actions.

Now, transitions have nothing to do with actions per se. A transition is not an action, it just uses an action.

For a lack of a better way to describe this, imagine 2 racing cars that are heading towards a finish line. A celebration even does not start until both cars have crossed the finish line. Those cars not in any way connected to each other, and each has its own action, being to move towards the finish line. There is only one way to determine that both cars have crossed the finish line, and that is by an external observer, not connected to the cars in any way. The racing car itself is not an action, the movement is, and one racing car crossing the finish line has nothing to do with another car crossing it, or what external event happens after they both cross.

Does that make sense?

[rh101]

I think WaitForAction would be more flexible, because it's an action and composes easily, something like:

Sequence::create(
    SomeAction::create(),
    WaitForAction::create(actionOnAnotherNode1, actionOnAnotherNode2, ...);
    SomeOtherAction::create()
);

With a CallFunc like thing you'd have to do this inside its callback. Also it would require inserting something into the action list and it would not work in situations when you can only get a pointer to an action like calling some Action* getSomeAction() method.

I strongly feel that everyone involved in this thread, and unfamiliar with the implementation of Action and sub-classes (particularly Sequence and FiniteTimeAction), should go through the code if you haven't already done so.

What is being suggested is not possible in the Sequence action, because it is not implemented how you may think it is; I had the wrong idea about it too until I went through the implementation to see how it was done, because I needed functionality somewhat similar to this quite a while back.

[rh101]

@AlexandreK38 Is this PR complete?

With regards to the discussion of the action system, perhaps that should be moved to a separate discussion.

[IamSanjid]

Just to clarify a few things. Firstly, there is nothing arbitrary about waiting for 2 unconnected actions (or action sequences) to finish, because in the case of scene transitions, there are only 2 scenes that each have their own actions applied. There can be no more than 2 scenes. These scenes are not connected in any way, and neither are there actions.

Now, transitions have nothing to do with actions per se. A transition is not an action, it just uses an action.

For a lack of a better way to describe this, imagine 2 racing cars that are heading towards a finish line. A celebration even does not start until both cars have crossed the finish line. Those cars not in any way connected to each other, and each has its own action, being to move towards the finish line. There is only one way to determine that both cars have crossed the finish line, and that is by an external observer, not connected to the cars in any way. The racing car itself is not an action, the movement is, and one racing car crossing the finish line has nothing to do with another car crossing it, or what external event happens after they both cross.

Does that make sense?

i get it why 2 is the number, it makes sense only through these discussion tho, i am saying the code isn't quite self explenatory to me(there could be other users like me with less experience) may be we should make comments for future users or may be we could just do something else like:

// Transition.h
// private enum class
enum FinishingScenes {
    None,
    InScene,
    OutScene,
    Other
}
FinishingScenes _lastFinishedScene = FinishingScenes::None;

// Transition.cpp
void finish(FinishingScenes currentFinishing = FinishingScenes::Other)
{
    if (_lastFinishedScene == FinishingScenes::InScene || _lastFinishedScene == FinishingScenes::OutScene)
    {
        // cleanup
    }
    _lastFinishedScene = currentFinishing;
}

//child transitions
_outScene->runAction(
        Sequence::create(/* custom params */, CallFunc::create(AX_CALLBACK_1(TransitionScene::finish, this, FinishingScenes::OutScene)), nullptr));
_inScene->runAction(
        Sequence::create(/* custom params */, CallFunc::create(AX_CALLBACK_1(TransitionScene::finish, this, FinishingScenes::InScene)), nullptr));

if we only care about current situation where the finish function will only be called by two actions, and we don't care what kind of actions are caling it, then setWaitForFinishCount(2) feels unncessary. just a different opinion, don't mind me.

[smilediver]

What is being suggested is not possible in the Sequence action, because it is not implemented how you may think it is

Oh right, I totally forgot that. :(

@AlexandreK38 Is this PR complete?

Not OP, but I think it is ready to be merged.

@IamSanjid: your example does not work, because finish() must detect when all relevant actions are complete, and not just one of in/out scene's. So you either need a counter, or some kind of flags for each finish signal.

[AlexandreK38]

@rh101 for me it is complete 🙏

At first IamSanjid idea may be better to explain what we do, but adding the scene param (and for example null when we don’t need to wait for several actions to finish or an enum using bitwise for flags and a setter similar to the setWaitFinishCount) isn’t simplifying what we do now

[rh101]

At first IamSanjid idea may be better to explain what we do, but adding the scene param (and for example null when we don’t need to wait for several actions to finish or an enum using bitwise for flags and a setter similar to the setWaitFinishCount) isn’t simplifying what we do now

The current changes in the PR are all that is required to fix the issue, and there is no need to over-engineer it; the implementation is both simple and very clear as to its purpose.

[IamSanjid]

@IamSanjid: your example does not work, because finish() must detect when all relevant actions are complete, and not just one of in/out scene's. So you either need a counter, or some kind of flags for each finish signal.

if the finish is called one after the other in the same thread,
say _outScene actions gets finished first then calls finish
_lastFinishedScene is default set to FinishingScenes::None so the cleanup block never reaches,
_lastFinishedScene gets set to FinishingScenes::OutScene,
Now _inScene actions gets finished then calls finish this time cleanup block should reach

[rh101]

if the actions are executed sequentially,
say _outScene actions gets finished first then calls finish
_lastFinishedScene is default set to FinishingScenes::None so the cleanup block never reaches,
_lastFinishedScene gets set to FinishingScenes::OutScene,
Now _inScene actions gets finished then calls finish this time cleanup block should reach

That is not correct, and there seems to be some misunderstanding regarding how the scene transitions work.

There are 2 separate nodes, each with their own actions applied. If you read through the code, you will quickly see that the actions are applied to the nodes at the same time, so the actions are running at the same time. They actions on one node are in no way linked to the actions of the other node. The order of actions is in not sequential between the 2 nodes (in and out scenes).

The finish condition is when the actions on both nodes are complete. The problem, and the initial reason for this PR by the OP, is that there is no way to control the order of completion of the actions, and there is no way for the order of completion to be determined. All that can be done is for the end of each action to somehow flag that it is complete, and once both are complete, then the finish condition is met. There is absolutely nothing wrong or confusing about the implementation made in this PR; it's quite literally a simple counter to track the completion of the transition sequence, and quite frankly, I do not see how it can be any simpler than that.

[IamSanjid]

if the actions are executed sequentially,
say _outScene actions gets finished first then calls finish
_lastFinishedScene is default set to FinishingScenes::None so the cleanup block never reaches,
_lastFinishedScene gets set to FinishingScenes::OutScene,
Now _inScene actions gets finished then calls finish this time cleanup block should reach

That is not correct, and there seems to be some misunderstanding regarding how the scene transitions work.

There are 2 separate nodes, each with their own actions applied. If you read through the code, you will quickly see that the actions are applied to the nodes at the same time, so the actions are running at the same time. They actions on one node are in no way linked to the actions of the other node. The order of actions is in not sequential between the 2 nodes (in and out scenes).

The finish condition is when the actions on both nodes are complete. The problem, and the initial reason for this PR by the OP, is that there is no way to control the order of completion of the actions, and there is no way for the order of completion to be determined. All that can be done is for the end of each action to somehow flag that it is complete, and once both are complete, then the finish condition is met. There is absolutely nothing wrong or confusing about the implementation made in this PR; it's quite literally a simple counter to track the completion of the transition sequence, and quite frankly, I do not see how it can be any simpler than that.

I don't get it what my example is doing wrong part? actions are executed simultaneously but not by different thread, my example doesn't care about order, it doesn't matter whether out scene first calls the finish function or in scene, it's always checks if the _lastFinishedScene is whether set to OutScene or InScene variant, when finish is first called by any of "in scene" or "out scene" Sequence the intial value _lastFinishedScene is always set to Other variant so the first finish always should ignore it's first call. And yes it was just a different opinion, I wouldn't say it's more simplified version it's almost the same, just the trying to understand my logic's mistake rn :)

[rh101]

I don't get it what my example is doing wrong part? actions are executed simultaneously but not by different thread, my example doesn't care about order, it doesn't matter whether out scene first calls the finish function or in scene, it's always checks if the _lastFinishedScene is whether set to OutScene or InScene variant, when finish is first called by any of "in scene" or "out scene" Sequence the intial value _lastFinishedScene is always set to Other variant so the first finish always should ignore it's first call.

Sorry, you're correct, I went through the code sample you provided again, and now I understand what it is doing. Yes, that would work, but, and this is just my opinion, it is not a simpler solution, nor is it easier to understand, than the solution implemented in this PR.

[IamSanjid]

I don't get it what my example is doing wrong part? actions are executed simultaneously but not by different thread, my example doesn't care about order, it doesn't matter whether out scene first calls the finish function or in scene, it's always checks if the _lastFinishedScene is whether set to OutScene or InScene variant, when finish is first called by any of "in scene" or "out scene" Sequence the intial value _lastFinishedScene is always set to Other variant so the first finish always should ignore it's first call.

Sorry, you're correct, I went through the code sample you provided again, and now I understand what it is doing. Yes, that would work, but, and this is just my opinion, it is not a simpler solution, nor is it easier to understand, than the solution implemented in this PR.

yeah the first misunderstanding by both @smilediver and you made it clear it's really not simpler or clearer, i don't know i just like to write code in more verbose way i guess, whenever i see something like setWaitForFinishCount(2) it's just feels like why the 2 shouldn't we be declaring what here 2 really means that's kind of stuff but may be that's just me.

[IamSanjid]

@rh101 for me it is complete 🙏

At first IamSanjid idea may be better to explain what we do, but adding the scene param (and for example null when we don’t need to wait for several actions to finish or an enum using bitwise for flags and a setter similar to the setWaitFinishCount) isn’t simplifying what we do now

btw about the null part just asking a question for future, do AX_CALLBACK_* macros automatically works with default arguments of a function right? so if finish had default value of it's argument we could still use it like AX_CALLBACK_0(TransitionScene::finish, this) right? or it won't going to work as if AX_CALLBACK_* are not aware of default function arguments? okay, this is getting too long and not worth anymore hope this gets merged after the answer peace.

[AlexandreK38]

@rh101 for me it is complete 🙏
At first IamSanjid idea may be better to explain what we do, but adding the scene param (and for example null when we don’t need to wait for several actions to finish or an enum using bitwise for flags and a setter similar to the setWaitFinishCount) isn’t simplifying what we do now

btw about the null part just asking a question for future, do AX_CALLBACK_* macros automatically works with default arguments of a function right? so if finish had default value of it's argument we could still use it like AX_CALLBACK_0(TransitionScene::finish, this) right? or it won't going to work as if AX_CALLBACK_* are not aware of default function arguments? okay, this is getting too long and not worth anymore hope this gets merged after the answer peace.

I never tried tbh, but we could give nullptr as param anyway. That was just an idea, I’m not gonna do this 🙂

About having many setWaitForFinishCount(2) we could replace by waitForSeveralFinishRequired() and set the count to 2 in the method as it’s always 2.

[rh101]

About having many setWaitForFinishCount(2) we could replace by waitForSeveralFinishRequired() and set the count to 2 in the method as it’s always 2.

That is not always the case. It can be 1 as well, such as for TransitionPageTurn.

EDIT: Never-mind, I see that _waitForFinishCount is initialized to 1, so the only time a method like setWaitForFinishCount(2) or waitForSeveralFinishRequired() would be called is if there are 2 scenes with actions. The current changes in this PR are good enough, using setWaitForFinishCount(), since its purpose is very clear.

[smilediver]

I think we're getting too fixated on this issue. We all agree that the fix could be improved with the right tools, but the tools are not available right now and current fix is ok. Let's just merge it. :)

[AlexandreK38]

About having many setWaitForFinishCount(2) we could replace by waitForSeveralFinishRequired() and set the count to 2 in the method as it’s always 2.

That is not always the case. It can be 1 as well, such as for TransitionPageTurn.

EDIT: Never-mind, I see that _waitForFinishCount is initialized to 1, so the only time a method like setWaitForFinishCount(2) or waitForSeveralFinishRequired() would be called is if there are 2 scenes with actions. I propose that you leave it as is, with setWaitForFinishCount(), since it's very clear as to its purpose.

Yeah let’s merge then 👍

[TyelorD]

Huh, our custom scene transition class that we wrote for our game inherits from axmol::TransitionScene, but we've never run into this issue with ordering (or if we have the issues have been so minor that we never noticed anything wrong).

Though now that I'm looking at the code I can see why. We only ever use a single action for the custom TransitionScene class itself, as well as a Spine Skeleton with some animations.

[AlexandreK38]

@halx99 sorry but did you forget to merge?