[solved] [BUG?] I don't expect Resume of views in stack when I assign Container.View

Bugs
1

So here is the situation:

I have some views in the stack, and I push/pop /set them when needed. All was good, until I wanted to place some logic to pause/resume, assuming when user sees the view it will start doing some game things (not reactive from input, but proactive), and best place for such behavior is Resume (as I think and hope, pls correct me if needed). Let’s imagine:

I have menu view, scenario selection view , game view, and gameview is in the viewstack, I click “to menu” button in game view, this pushes menu view on top of game view, and I show “back to game” button in menu ( when I have not finished “game session”). Now I have 2 options - return to game (player wants to continue) or I can start fresh (and go to new scenario). First is trivial push and works fine, as game logic is the same. But if I go for new game, I click new scenario, which reinintializes the game logic , and does Container.View := ViewScenario; thus removing all the views from the viewstack. That’s my logic, kinda intuitive , I think.

But what’s happening in reality:

Container.View := ViewScenario; does make

while ViewStackCount <> 0 do

    PopView;

and PopView does FViewStack.Last.Resume; for really short moment (the cycle is intended in general to remove all the views this way with Pop-s), but at this moment gameView thinks it got the timeslot and makes some game things within the old game state (but it is now reinited).

So, the question is, does it really necessary to call resume from this while-loop ? it doesn’t seem so to me.

I did a workaround in ancestor view-class in my project for the SetView process:

DontFireResumeOnSetView := True;

    Container.View := nil;

    DontFireResumeOnSetView := False;

    Container.View := AView;

and the descendant view who wants to protect itself from this “technical” not real Resume, just does in Resume:

if DontFireResumeOnSetView then Exit;

Your thoughts are welcome :slight_smile:

2

I see the issue and I agree it’s a bug. I’ll look into it (around this weekend, I’ll let here know).

Thanks for the analysis, all agreed. The loop while ViewStackCount <> 0 do PopView; does (right now, before fix) make resume on all views (except front-most) before pausing and stopping them. This in unintuitive and just undesired side-effect of this loop. We should just stop the non-front-most views, without a pointless resume+pause :slight_smile:

I will see to fix it, I need to do it carefully and auto-test :slight_smile: I will have to clarify (and autotest) what is Container.FrontView temporarily during Container.View := ... assignment (in case something is watching it), in the new approach – it could be temporarily a paused view (that is going to be stopped right away). Of course after Container.View := ... assignment finished, Container.FrontView will reflect the new view.

1 Like
3

Ok, glad that I’m able to help, at least this way :slight_smile:

Is it worth to create a bug ticket in github ? Not sure if I’m able to address the topics that you describe, if I would make a PR (I would probably just introduce a flag for PopView process to skip Resume).

4

GitHub issue or PR would be welcome, sure.

Note that it’s not enough to skip Resume in TCastleContainer.PopView – it should also skip pausing the new top-most view (to avoid pausing an already-paused view). Current TCastleContainer.PopView logic assumes that FrontView is always active, but this assumption will no longer be true.

I would advise to activate TCastleView.Log and watch the Start/Resume/Pause/Stop calls – they should match (one Pause for one Resume, one Stop for one Start). Maybe that’s also a good automated test,

  • create a TCastleView descendant
  • make it count the start/stop and resume/pause calls, to make sure they always match,
  • and BTW, make sure resume/pause happen only between start/stop.

Anyhow, any progress is welcome, thank you in advance :slight_smile: If you will make a PR which will fix the issue for you, it will be a useful start for me even if I will want to adjust there something before merging.

1 Like
5

Fixed! In commit Do not make unnecessary Resume+Pause for non-top-most views on stack … · castle-engine/castle-engine@dfafe7d · GitHub .

Along with auto-test that all “lifecycle” methods work as expected.

Thank you for reporting!

1 Like