replace() doesn't add the replaced screen onto the stack
Jerakin opened this issue · 11 comments
Calling replace doesn't seem to add the replaced screen on the stack
monarch.show("game") -- "Normal screen"
monarch.show("menu") -- "Popup screen"
print(monarch.dump_stack())
monarch.replace("score") -- "Popup screen"
print(monarch.dump_stack())
The print output is this
DEBUG:SCRIPT: 1 = hash: [game]
2 = hash: [menu]
DEBUG:SCRIPT: 1 = hash: [game]
When I call monarch.replace twice in a row the second time the screen doesn't show. By explicitly saying pop=0 I can get the item to show. Though I still do not get the monarch.dump_stack() I expect.
Trying to use replace() to "cycle" screens.
Expected outcome:
Screen A is loaded [Stack: A]
Popup B is loaded [Stack: A, B]
Popup C replacing B [Stack: A, C]
Popup D replacing C [Stack: A, D]
Actual outcome:
Screen A is loaded [Stack: A]
Popup B is loaded [Stack: A, B]
Popup C replacing B [Stack: C]
UNLOADS C BUT NEVER LOADS D [Stack:]
Ok, so first of all there was a a problem in the code where an error wasn't caught and handled properly. This has been fixed. BUT the new replace functionality is a bit tricky:
When show() is called the first thing that happens is that any open popups are called, unless the new screen is also a popup and the "popup on popup" option is enabled on the current popup. The replace functionality comes into play first AFTER the popups are closed. This results in:
- Stack contains A,B
- Calling monarch.replace("C") (ie pop = 1)
- B is a popup -> close it!
- Stack contains A
- Now replace/pop comes into play and removes A from the stack (but doesn't unload or hide it)
- C is shown
- Stack contains C
So the question is what the expected behavior is when popups exist? I mean, say you have a screen and then two popups in the stack: [S1, P1, P2]. What happens if you replace/pop with a normal screen? You can't expect to have [S1, P1, S2]. The only logical thing is for popups to first close and then replace which in the example would mean [S2] in the stack.
But it also means that using replace() with popups is a bit weird:
[S1, P1] - replace(P2) -> [P2]?
Replace is the normal behavior when showing one popup on top of another (unless popup on popup is allowed).
Thoughts on this @dapetcu21 ?
Yes, I agree. In the case where you're replacing with another screen, we should definitely do the replace after all the popups are closed.
As you pointed out, when you're replacing with a popup with popup on popup enabled, the situation is a bit tricky. We could either leave it as is, or I propose we do the following:
Treat the popup and screen sections of the stack separately.
If the screen that is to be shown/replaced is a popup, then do all the pops off the stack (if they're more than the number of popups on the stack, then stop), THEN do the top.popup and screen.popup_on_popup check and pop all the popups if necessary, THEN push the new screen on the stack.
If the screen that is to be shown/replaced is a screen, then pop all the popups, then do all the pops, then push the new screen.
Some examples of that behaviour:
[S1, P1] - replace(P2) -> [S1, P2]
[S1, P1, P2] - replace(P3, popup_on_popup) -> [S1, P1, P3]
[S1, P1, P2] - replace(P3) -> [S1, P3]
[S1] - replace(P1) -> [S1, P2] // There are no popups here, so replace() acts like show()
[S1, P1, P2] - replace(S2) -> [S2]
Yes, this is probably the way forward to properly handle all cases.
I'll do a PR, then
If you have the time that would be great!
Can confirm that replace now works as I presumed it would! 👍
I noticed today that if I do .replace from within the script that gets replaces the new screen does not get onto the stack (but it still is loaded/shown).
Example: [S1, P1] - replace(P2) -> [S1, P2]
So if I in P2 do replace(P2)
The stack will become
[S1] instead of the expected [S1, P2]
I'm trying to reproduce this in the example that ships with Monarch by modifying the two popups "about" and "confirm":
[menu] -> menu.gui_script calls show(about) -> [menu, about] -> about.gui_script calls replace(confirm) -> [menu, confirm] -> confirm.gui_script calls replace(confirm) -> [menu, confirm]
If I understood your bug report it would mean that when the Confirm popup is on the top of the stack and when confirm.gui_script calls replace("confirm") it would get removed but not added back.
Could you please create a small example?
Not able to reproduce and no sample project provided. Closing.