Don't exit on losing all lemmings (feature development)

[kaywhyn]

@Simon

Correct, that is a very bad backroute for my R3 - Fun Teleportation Race in Space as you have guessed. Good find! I'll need to think of a fix for the level. I will release an update when the LOTY2023 playing phase starts in a few weeks or so.

[WillLem]

I dumped the exp 3's config into the exp 4, then played without looking at the exp 4's options dialog. NL freezes both if won (wrong) and if lost (correct). But if we look into the options dialog, the middle option (Exit to Postwiew if Save Requirement Met) is selected. This doesn't match what we just saw in play

This is a difficult bug to replicate, and from what I can see the flag is being loaded from and saved to config correctly. Outputting to debug also indicates that the flag is set correctly when gameplay starts. I might need a second pair of eyes on this one.

It still overshoots by one frame ... Sometimes it doesn't overshoot

Yes, I can replicate this. Best guess: it takes a tick to register the flags (but this wouldn't explain why it sometimes doesn't happen). Unsure exactly how to solve this tbh.

If you win with the final lemming that goes into the exit, and there are no more lemmings ... NL tells you: No more lemmings, rewind or quit. NL won't quit here even though I've won

Again, having a hard time replicating this. In all tests I performed, levels always exit to postview correctly.

We check for the option flag and the rescue count being equal to or more than the requirement. Here it is, maybe you can see if there's something wrong?

Code: [Select]

// End of gameplay options for when no lemmings remain
  if ((Level.Info.LemmingsCount + LemmingsCloned - fSpawnedDead) - LemmingsRemoved = 0)
    and (DelayEndFrames = 0) and not UserSetNuking then
  begin
    if GameParams.AlwaysEndGameplay
      or ((LemmingsIn >= Level.Info.RescueCount) and GameParams.EndIfLevelPassed)
        then ExitToPostview
    else if GameParams.NeverEndGameplay
      or ((LemmingsIn < Level.Info.RescueCount) and GameParams.EndIfLevelPassed)
        then begin
          if not ShowNoLemsMessage then ShowNoLemsMessage := True;
          Exit;
        end;
  end;


[Simon]

"I dumped the exp 3's config into the exp 4": This is a difficult bug to replicate, and from what I can see the flag is being loaded from and saved to config correctly. Outputting to debug also indicates that the flag is set correctly when gameplay starts. I might need a second pair of eyes on this one.

I've attached my settings.ini from the exp 3.

Ideas: Assuming you load, save, and use the option properly, there is a fourth thing to it: What happens to this option when we start NL with empty options file/new installation/...?

If you win with the final lemming that goes into the exit, and there are no more lemmings ... NL tells you: No more lemmings, rewind or quit. NL won't quit here even though I've won

tests I performed, levels always exit to postview correctly.
maybe you can see if there's something wrong?

When exactly do you call into the code that you showed?

When does NL update LemmingsIn? Reason: In my vod at 02:00:45 (the same time that I wrote into my previous post), we clearly see 0 flagpole and the message. According to your code, this can only happen when LemmingsIn isn't 10 yet, and the level requires to save 10/10. Do you call into your code too early?

What is DelayEndFrames? Do you expect it to be 0 in my vod around 02:00:45 when I run into the bug?

Best guess: it takes a tick to register the flags (but this wouldn't explain why it sometimes doesn't happen)

Correct, this doesn't explain why it sometimes freezes correctly (instead of overshooting by 1 frame).

But you make it sound as if you test for end-of-game during the physics updates, not in-between the physics updates. It's okay if you call it after a physics update and cache your results, but still, caching means more mutable state, and mutable state is a source of bugs.

Consider to factor this code into two methods: One to test for end-of-game (regardless of how we react to that), and one to tell you whether we should show the message (that, among other things, asks the end-of-game method). In effect, try to refactor the flag ShowNoLemsMessage into a method, to avoid mutable state, and call this method where you would have read the flag.

-- Simon

[WillLem]

What happens to this option when we start NL with empty options file/new installation/...?

Same thing as with every other option: a default is set (in this case, Always Exit To Postview).

When exactly do you call into the code that you showed?

It's a check that happens every update (Update Lemmings / CheckForGameFinished), to see if the end-of-gameplay conditions have been met.

When does NL update LemmingsIn?

UpdateLemmings / CheckLemmings / CheckTriggerArea / HandleExit / HandleExiting / RemoveLemming(Saved) / Inc(LemmingsIn)

So, every update. However, within UpdateLemmings, CheckLemmings is called after CheckForGameFinished. So, in theory, that might explain why it takes another update to set everything correctly and why the game didn't exit properly when you saved the last lem. If we move the checks around though, it may give rise to other bugs...

What is DelayEndFrames? Do you expect it to be 0 in my vod around 02:00:45 when I run into the bug?

DelayEndFrames specifically waits for trap animations to finish.

It's okay if you call it after a physics update and cache your results, but still, caching means more mutable state, and mutable state is a source of bugs.

Consider to factor this code into two methods: One to test for end-of-game (regardless of how we react to that), and one to tell you whether we should show the message (that, among other things, asks the end-of-game method). In effect, try to refactor the flag ShowNoLemsMessage into a method, to avoid mutable state, and call this method where you would have read the flag.

[Simon]

within UpdateLemmings, CheckLemmings is called after CheckForGameFinished.

Advancing physics has nothing to do with deciding whether we have reached dead state.

Reason: When we have reached dead state, you don't even begin advancing physics. You can decide about whether a state is dead well after advancing physics. In no way advancing physics depends on being dead; originally, in your pause design, you considered continuing into dead state.

Indeed, move the test for dead state out of advancing physics altogether. Fear not!

cat

Your code does: Test for dead state once per update and set some variables (e.g., ShowNoLemsMessage). Later, read these variables.

You should do: Remove the variables. Later, in the code that needed to read them, instead re-test for dead state.

-- Simon

[WillLem]

Indeed, move the test for dead state out of advancing physics altogether

The physics freeze is achieved by exiting from the update procedure (i.e. the physics advance), so some check needs to be performed at the start of the update procedure. Maybe I've misunderstood?

Remove the variables. Later, in the code that needed to read them, instead re-test for dead state.

We need to achieve 6 goals:

1) Define the dead state (no active lems, no animations that need to finish, SR isn't met)
2) Prevent physics updates when the dead state is reached
3) Prevent overshooting into the dead state
4) Show a message informing the player
5) Allow the player to rewind into the live state
6) Allow all of the above to happen again after rewinding

The various checks implemented so far are to make sure that all of the above happens. I tried what you suggested, and it broke goals (5) and (6), also meaning that testing for (3) became impossible.


Meanwhile, the config changed slightly between Exp 3 and 4 (notably, the Freeze option was removed) - it's possible that this had something to do with the settings bug you encountered...

Anyways, I placed your Exp 3 settings file into a fresh copy of Exp 4 and the game behaved as expected: exit to postview for a win, freeze for a loss. I also had difficulty replicating the "one frame overshoot" bug when testing for this as well. I'm not sure why it sometimes happens and sometimes doesn't - this makes it particularly difficult to identify the cause!

[WillLem]

Happily, whilst checking for possible solutions to the bugs reported by Simon, I discovered that it's not actually necessary to call the update procedure again when nuking; nuke still acts as a "cancel" button when the freeze state is entered.

We also now check for (GameFinished or CheckForUnplayableState) at the same time, at the beginning of the update procedure. This doesn't fix the single frame overshoot, unfortunately, but it seems to make more sense and it might prevent the bug Simon encountered when playing kaywhyn's level (i.e. the game froze when the final lemming exited, even though it was a win).

NOTE: I tried moving CheckForUnplayableState to GameWindow's Application_Idle instead of LemGame's UpdateLemmings. It made no difference; overshooting by 1 frame still happened. So, it's moved back to UpdateLemmings (again, it makes more sense in the context of this feature's wider implementation - ideally, we don't want GameWindow to deal with any of this, but it's currently necessary to prevent overshooting with large frame skips).

I've attached the updated copies of GameWindow and LemGame here.

Methinks, a screen-sharing session is in order to try and get to grips with the bugs. Simon?

[Simon]

It sounds like you're still setting variables, whose contents then go out of date and you have those bugs as in broken (5) and (6), instead of retesting for dead state whenever necessary.

Move the code for (1) into its own function and call it from several places if necessary. No caching! Re-test every time!

Do you have a public repository? Can you show me git history until here? Reason: That (5) and (6) are even a concern makes me wonder about the architecture. If you don't have history, I'll have to work with those 7,000-line files. Possible, but takes longer to figure out what's going on.

Screensharing: Do you have tooling for it?

-- Simon

[WillLem]

Happily, whilst checking for possible solutions to the bugs reported by Simon, I discovered that it's not actually necessary to call the update procedure again when nuking; nuke still acts as a "cancel" button when the freeze state is entered. No, it turns out I was wrong about this, the nuke hack is currently still necessary - I'll try to work out a better way to do this.

We also now check for (GameFinished or CheckForUnplayableState) at the same time, at the beginning of the update procedure. This doesn't fix the single frame overshoot, unfortunately, but it seems to make more sense and it might prevent the bug Simon encountered when playing kaywhyn's level (i.e. the game froze when the final lemming exited, even though it was a win).

NOTE: I tried moving CheckForUnplayableState to GameWindow's Application_Idle instead of LemGame's UpdateLemmings. It made no difference; overshooting by 1 frame still happened. So, it's moved back to UpdateLemmings (again, it makes more sense in the context of this feature's wider implementation - ideally, we don't want GameWindow to deal with any of this, but it's currently necessary to prevent overshooting with large frame skips).

I've attached the updated copies of GameWindow and LemGame here.

Methinks, a screen-sharing session is in order to try and get to grips with the bugs. Simon?

[WillLem]

Move the code for (1) into its own function and call it from several places if necessary. No caching! Re-test every time!

Yep, this is exactly how the feature is implemented. CheckForUnplayableState is used for both the physics freeze and the display message, and it gets its info from various sources including UpdateLemmings and CheckForGameFinished.

Do you have a public repository? Can you show me git history until here?

Bitbucket repository / commits - let me know if you need permissions.

Screensharing: Do you have tooling for it?

Discord preferably, Zoom if there's no other option.

[Simon]

Okay, I'm cloning the repo. I'll have ~1 hour free now; I'm sitting in Mumble, hop on Mumble if you see this and are free.

I'll make my mind up about Discord or Zoom, both are nonfree with insidious privacy policies.

-- Simon

[WillLem]

I'm sitting in Mumble, hop on Mumble if you see this and are free.

Busy now unfortunately, later tonight or sometime tomorrow?

[Simon]

Tonight 21:00 UTC (= 22:00 my local time), yes. Otherwise tomorrow, anything between 17:00 and 24:00 UTC as it suits you.

Yes, you have a method CheckForUnplayableState for the unplayable state, good. Now, across the entire LemGame.pas, remove the variable fShowNoLemsMessage and its getter/setter entirely. As a result, in CheckForUnplayableState, you'll have one fewer condition that gets and-ed together. That should remove those state-caching bugs. Does it introduce any new bugs?

-- Simon

[WillLem]

@Simon - I’ll be there in 5 mins

[Simon]

We've reviewed code via Mumble and git.

WillLem had the factoring practically right even before I looked at it. There were already no bugs with the rewind-or-restart message.

We found and fixed the overshooting. The overshooting came from leftover caching information from end-of-game considerations at the start of each physics update. Such end-of-game tests must either happen at the end of a physics update, or outside the physics update. We still have to reconcile the nuke with this; the nuke shall ignore the unplayable state and continue into nuking zombies, then exit. Because of this, we'd rather not tie these decisions to the physics update -- after all, activating the nuke to continue is UI, not physics, and only then it adds the nuke entry to the replay and nuke in the physics.

NL's 2023 architecture tied the exiting to the start of a new physics update. In light of our feature -- stop updating, but don't exit on losing all lemmings -- it looks necessary to change the exiting architecture in some way; still, we should review it with namida on a high level. For now, we renamed CheckForGameFinished to MaybeExitToPostview, to distinguish it clearly from the new end-of-game behaviors that don't exit, and moved it out of the physics update, into the window's main loop, as a public method call to LemGame.

WillLem has tested the interactive game and found no problems with the moved exit-to-postview decision. We haven't yet tested mass replay verification, we'll still have to do that. Likely, it will work because the verification carries its own exiting logic.

Thus, to do:

• Reconcile nuke-to-exit with the moved exiting logic.
  
• Test mass replay verification. Most likely, it's fine.
  
• Test other ways to exit LemGame, e.g., pressing Esc, Hyper Speed into death/win.
  
• Fix my observed mismatch between default option and game exiting.
  
• Stream more playtesting!
  
• Review the high-level achitecture of the feature with namida.
  

-- Simon