Skip to content

Add support for extracting the previous scene from the scene stack #2793

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.

Sign up for GitHub

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

Jump to bottom
Merged
halx99 merged 3 commits into from
Sep 23, 2025
Merged

Add support for extracting the previous scene from the scene stack #2793

halx99 merged 3 commits into from
Sep 23, 2025

Conversation

@rh101
Copy link
Contributor

@rh101 rh101 commented Sep 23, 2025 •
edited
Loading

Describe your changes

This PR adds support for extracting the previous scene from the scene stack. This would allow transitions from the current scene to the previous scene, such as:

auto director = Director::getInstance();
auto previousScene = director->popPreviousSceneOut();
auto transition = Transition::create(2, previousScene);
director->replaceScene(transition);

Issue ticket number and link

Checklist before requesting a review

For each PR

  • Add Copyright if it missed:
    - "Copyright (c) 2019-present Axmol Engine contributors (see AUTHORS.md)."

  • I have performed a self-review of my code.

    Optional:

    • I have checked readme and add important infos to this PR.
    • I have added/adapted some tests too.

For core/new feature PR

  • I have checked readme and add important infos to this PR.
  • I have added thorough tests.

Axmol 3.x ------------------------------------------------------------

For each 3.x PR

  • Check the '#include "axmol.h"' and replace it with the needed headers.

@halx99 halx99 added this to the 2.9.0 milestone Sep 23, 2025
@halx99
Copy link
Collaborator

halx99 commented Sep 23, 2025

API naming: what about popPreviousScene or popPreviousSceneOut ?

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025 •
edited
Loading

API naming: what about popPreviousScene or popPreviousSceneOut ?

I was considering something popPreviousScene earlier, and my only concern was that there would be confusion at the difference in behavior between that and the similarly named popScene().

Maybe popPreviousSceneOut would work, since it implies taking it out of the scene stack? Are you happy with that one?

@halx99
Copy link
Collaborator

halx99 commented Sep 23, 2025

Maybe popPreviousSceneOut would work, since it implies taking it out of the scene stack? Are you happy with that one?

lgtm

@halx99 halx99 added the enhancement New feature or request label Sep 23, 2025
@aismann
Copy link
Contributor

aismann commented Sep 23, 2025

@rh101
What is the different to popScene()

/**
 * Pops out a scene from the stack.
 * This scene will replace the running one.
 * The running scene will be deleted. If there are no more scenes in the stack the execution is terminated.
 * ONLY call it if there is a running scene.
 */
void popScene();

@halx99
Copy link
Collaborator

halx99 commented Sep 23, 2025 •
edited
Loading

Maybe update the comment of the popScene (scene just removed)

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025 •
edited
Loading

@rh101 What is the different to popScene()

/**
 * Pops out a scene from the stack.
 * This scene will replace the running one.
 * The running scene will be deleted. If there are no more scenes in the stack the execution is terminated.
 * ONLY call it if there is a running scene.
 */
void popScene();

As the comment indicates, popScene() deletes the last scene from the stack (which is the current scene), and then switches to the next scene in the stack, so on the next update cycle it becomes the current scene.

This new method in this PR, popPreviousSceneOut(), removes the second-last scene on the stack, and returns it to the caller. The caller can either choose to ignore it, so it will be released/freed on next cycle, or use it to allow an effect like a transition sequence to be made between the current scene and that removed scene.

At the moment you can go from current scene to new scene and apply a one of the Transition implementations to it, but you cannot go the other way around. This PR adds that functionality to allow you to transition back to the previous scene.

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025 •
edited
Loading

Maybe update the comment of the popScene (scene just removed)

I'll update the comment for it now to clarify that.

Actually, the comment seems fine:

    /**
     * Pops out a scene from the stack.
     * This scene will replace the running one.
     * The running scene will be deleted. If there are no more scenes in the stack the execution is terminated.
     * ONLY call it if there is a running scene.
     */

It states exactly what happens:

  1. Pop scene from stack
  2. Popped scene replaces current running scene
  3. Running scene is deleted

Is there something about it that is confusing?

@aismann
Copy link
Contributor

aismann commented Sep 23, 2025 •
edited
Loading

Maybe update the comment of the popScene (scene just removed)

I'll update the comment for it now to clarify that.

Please have a short look on the other comments too.

@aismann
Copy link
Contributor

aismann commented Sep 23, 2025

At the moment you can go from current scene to new scene and apply a one of the Transition implementations to it, but you cannot go the other way around. This PR adds that functionality to allow you to transition back to the previous scene.

@rh101
How about a little tester? Isn't that useful?

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025

Please have a short look on the other comments too.

All comments make sense. Is there something specific about any of the comments that is causing confusion?

@aismann
Copy link
Contributor

aismann commented Sep 23, 2025

Please have a short look on the other comments too.

All comments make sense. Is there something specific about any of the comments that is causing confusion?

I'm not sure. I'm not a native English speaker.
You can ignore it.

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025 •
edited
Loading

@rh101 How about a little tester? Isn't that useful?

A test may not be useful or required for this, since it's a very straightforward function to use; it simply pulls the second-last scene from the list (the 2nd scene in the stack).

If anyone feels that a test is required, then I can add one, but it may not be straightforward due to how the Scene Test section is implemented.

@aismann
Copy link
Contributor

aismann commented Sep 23, 2025 •
edited
Loading

A test may not be useful or required for this, since it's a very straightforward function to use; it simply pulls the second-last scene from the list (the 2nd scene in the stack).

Thank you for clarifying that.

@aismann aismann closed this Sep 23, 2025
@aismann aismann reopened this Sep 23, 2025
@aismann
Copy link
Contributor

aismann commented Sep 23, 2025

Sorry I pressed the wrong button

@rh101
Copy link
Contributor Author

rh101 commented Sep 23, 2025 •
edited
Loading

Sorry I pressed the wrong button

To avoid wasting resources, I cancelled the new run of the checks that were triggered by closing and re-opening this PR, since it's already completed in: https://github.com/axmolengine/axmol/pull/2793/checks?check_run_id=51005811514

@aismann
Copy link
Contributor

aismann commented Sep 23, 2025

Sorry I pressed the wrong button

To avoid wasting resources, I cancelled the new run of the checks that were triggered by closing and re-opening this PR, since it's already completed in: https://github.com/axmolengine/axmol/pull/2793/checks?check_run_id=51005811514

Thanks. Sorry again.

@halx99 halx99 merged commit ae71a00 into axmolengine:release/2.x Sep 23, 2025
15 of 29 checks passed
@rh101 rh101 deleted the prev-scene branch September 23, 2025 07:46
tkzcfc pushed a commit to tkzcfc/axmol that referenced this pull request Sep 23, 2025
Merge branch 'release/2.x' into dev_custom
9ded397 
* release/2.x:
  Fix ios editbox keyboard handling (axmolengine#2795)
  Add support for extracting the previous scene from the scene stack (axmolengine#2793)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@halx99 halx99 halx99 approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

2.9.0

Development

Successfully merging this pull request may close these issues.

3 participants

@rh101 @halx99 @aismann