[SUG] Improve "same-frame assignment to >1 lem" behaviour in Replay Insert mode

[WillLem]

From this topic - the bug being reported is that assigning a new skill in "Blue R" (Replay Insert) mode silently overwrites any other skill assignments to other lems on that frame. I usually get around it by framestepping forwards and backwards to make sure I'm not going to overwrite an assignment, but - when I think about it - that's a pretty naff thing to have to do.

So, my thoughts are that one of 2 things should happen here, and one of them will be added to the SuperLemmix wish list in due course:

1) The old assignment should be detected, and the new assignment shouldn't be allowed. Now that we have the Assign Fail sound, this sound can be used here to signify that there is some reason the assignment can't be performed on that frame. An onscreen graphic saying "there is already an assignment on this frame" could also be helpful.

2) Both assignments should be allowed. Since Namida has described this as a "change of ... magnitude", I imagine that either it's something that will take a lot of time and effort to implement, or something that could open up the engine to a lot of bugs, or something that just isn't easy to figure out how to do. Since I'm now more familiar with the codebase, I know a bit more about how the replay system works, but not enough to take on something like this without a bit of help.

So, it's really a case of - which do we prefer?

If the former, I can't imagine that being too difficult to set up. Create a new flag whenever a successful assignment is made, and then check for that flag when making new assignments in Replay Insert mode - there's likely a bit more to it than that, but I at least have some idea how to get started.

If the latter, then I'll need help looking at it and figuring out what the first step is. Maybe Simon and I can add it to the list of things we may look at together? Mayba Namida won't mind casting a quick eye over it - if you can get me started, I'll figure out the rest?

Meanwhile, I do need to find ways to make some of the game features a bit more prominent. Onscreen graphics is one way to achieve this, but I'm having a lot of trouble figuring out exactly how to do this. Windows "Show Message" is easy enough, but not very video-gamey. I'll keep looking into it, because there are now several aspects of the game which probably need it.

[Simon]

In this post, I'll assume that SLX will allow simultaneous assignments. Reason: Allowing simultaneous assignments feels like the saner game design when we have powerful inserting/tweaking features.

The first design problem is to decide what the data structure (that is in the replay, that holds all assignments) must offer.

• Do you want to remember an ordering among several same-frame assignments?
• Do you want to allow same-lemming-same-frame assignments in the replay? (This is separate from whether the UI, on assignment, first cuts same-frame-and-future of the same lemming.)
  

Same-lemming-same-frame can be useful when you eventually add better tweaking, and it so happens that same-lemming assignments temporarily are on the same frame during tweaking. I haven't thought about it much yet.

Lix just allows it all in the replay, and it remembers the ordering. (Reason: Same-lix assignments may come from the network, from different players on the same team.) I allow extreme cases such as 50 bashers, then 20 builders, then 10 floaters, all to the same lix in the same physics update. The result is that the player loses 1 basher, then 49 bashers fail to assign because you can't assign basher to basher, then we start building and queue 19 more builders, and then it becomes a floater and 9 floater assignments fail. Only the basher goes to waste. It's still stange for sure, but this is only about what the data structure should offer. We can solve some problems with good UI, and it's rarely a problem in practice anyway. (The biggest problem in practice is junk assignments that fail after tweaking, and that problem arises independently of (dis)allowing same-lemming-same-frame assignments in the replay.)

Down the road, you'll have to touch the code that reads assignments from the replay and applies them to the physics. Now, with zero-or-one assignments per physics update, NL call that code either once per physics update or not at all in that physics update. With multiple assignments, you'll call it several times per physics update, once for every assignment, in the correct order.

It's possible that NL relies elsewhere on identifying the replay's assignments by frame. Down the road, the frame number won't be unique anymore.

-- Simon

[WillLem]

Yead, I'd need help with that...

Let's start with just allowing same-frame assignments to different lems, then - if that works without too many headaches/issues - we can look at same-lem assignments.

Long-term plans are for SLX to have multiplayer, but I'm more likely to make it 2P only (at least to begin with). So even then, multiple assignments to the same lem are less likely to happen/be a problem.

[WillLem]

OK, there are no fewer than three separate functions/procedures which handle skill assignment (namely: AssignNewSkill, DoSkillAssignment and ProcessSkillAssignment). There is also a procedure specifically for writing assignments to the Replay (RecordSkillAssignment).

Happily, we already have a check for assignments on the current frame (fDoneAssignmentThisFrame), which returns true when an assignment is successfully made, and false on a normal physics update frame.

I'm getting into a bit of a headspin trying to figure out exactly how it all works, but I'm pretty sure it's all right here in front of me.

So far, I've achieved the following:

I can prevent assignments at any time when using a check for Replay Insert mode by itself - so far so good. However, deploying the aforementioned fDoneAssigmentThisFrame check only prevents assignments on the next consecutive frame (i.e. not the same-frame) after a successful assignment.

Best guess: the game is trying to make the assignment on that next frame before the physics update is actually made, so the check is still returning true. Meanwhile, the check is false on the frame immediately before the assignment, so assignments to any lem are still possible at that point. Here's a graphic to help show what's happening:



So, this isn't much use, as it turns out. What's needed is a way to check for an assignment on the next frame...

Also, it turns out that ReplayInsert mode is never cancelled once started, unless it's manually cancelled by the player (at least, that's what the Debugger says). This can't be the case, though, because the "R" disappears from the panel. This is, unfortunately, one of those mysteries that this codebase often presents.

I could really use a second pair of eyes on this. I'm pretty sure we can have a solution to this problem pretty quickly.

[Simon]

Best guess: the game is trying to make the assignment on that next frame before the physics update is actually made

Yes, that should be how it works.

• Game is paused, wait for input between physics updates n and n + 1.
  
• It's time to advance physics, i.e., to perform physics update n + 1.
  
• Give the assignments for n + 1 to the lemmings. (Can be 0 assignments, 1 assignment, or, as you envision, more).
  
• Perform the remaining parts of the physics update that always happen regardless of assignments.
  
• Game is paused, wait for input between physics updates n + 1 and n + 2.
  

While the game is between physics updates n and n + 1, what is the current physics update? n or n + 1? Between which two of my items does the game increase this number?

flag is true
So, this isn't much use, as it turns out. What's needed is a way to check for an assignment on the next frame...

Have fewer flags. Mutable state is a source of bugs; we want to minimize the amount of mutable state. Instead, write functions that compute the information on demand from replay content, and from necessary-to-have mutable state (number of the physics update).

Last time I looked, the NL game class was big. See what methods you can move from the game class into the replay class (I assume NL has one), e.g., fetch all assignments for a given frame. Then you can call it with whatever frame number that interests you, the current, or the next.

-- Simon

[WillLem]

While the game is between physics updates n and n + 1, what is the current physics update? n or n + 1? Between which two of my items does the game increase this number?

I think IncrementIteration (in LemGame) deals with advancing the frames. But the physics updates themselves are all handled by UpdateLemmings (LemGame) and Application_Idle (GameWindow).

Meanwhile, we have some progress with this topic - I've figured out how to prevent same-frame assignments! Yes, this isn't the ideal eventual goal, but it's an improvement over the current behaviour of silently overwriting existing assignments, so it will do for now whilst we look for a way to make the ideal happen.

It turns out that the answer lies with the Replay Manager's handling of assignments, rather than the game itself's handling. The Replay Manager already (thankfully) has a way to check for assignments on past, current and future frames. This check is always looking at the existing replay file into which previously-made assignments have already been written, recorded and stored, whereas the aforementioned fDoneAssignmentThisFrame flag doesn't interact with the replay at all, and so has no way of knowing or responding to this information.

So, you were absolutely correct with this:

Have fewer flags ... Instead, write functions that compute the information on demand from replay content, and from necessary-to-have mutable state (number of the physics update).

Turns out such computations already exist within the codebase, thankfully! It was a case of checking the Replay Manager itself for an assignment at the current frame. The existing check looks for Assignments and RR changes, so I duplicated it and made a check for Assignments only (I previously separated out RR changes for a bugfix elsewhere*, so we now have checks for "both", "only Assigments" and "only RR changes").

It works because, somewhat paradoxically, the game needs a way to look into the future for a change on that frame whilst it's backstepping, because backwards skips go further back before heading towards the target frame at Hyper Speed (what a great sentence that was to type! ). The game itself can only deal with whatever is currently true at that frame, whereas the Replay Manager holds the key for checking past and future events.

So, the "don't overwrite, but also don't accept new assignments" behaviour will be present in the next version of SLX (2.4). The "assign fail" sound plays as well, so it's doubly clear that the assignment can't be made (even if it's not obvious why). Commit 760e513ff implements this.

(If we find a way to accept multiple assignments in the meantime, then of course this will be updated again before release).


*I used the same check in order to fix the bug that caused the RR sound to trigger during backwards skips. The sound was triggering because there had been a change between Frame 0 and Current Frame, so I needed to say "only trigger the sound if there's a change on the current frame. So, I separated out this part of the code and made a new check accordingly.

[WillLem]

OK, so I'm beginning to look at ways to allow same-frame-different-lem assignments. I agree, this is the best design choice when we have Replay Insert and Editing capabilities. I even think such Replays ought to be play-back-able in Classic Mode, even if they're not creatable there (Replay features are not available in-game, but Replays can be loaded from the preview screen, when in Classic Mode).

Having slept on it, and reading back over the posts, I'm thinking that for now it's preferable to just not allow same-lem-same-frame assignments rather than get into potential headaches with the Replay structure. The only time I can see it being relevant is in a team-game Multiplayer scenario (as outlined by Simon here), which SLX is a long way off getting, if ever.

For Multiplayer, I'll far more likely start off with 2-Player battle mode, and might even just leave it there depending on usage/popularity. I like the idea of creating a co-op mode as well, in which the players work together to solve the puzzle or traverse the level map, but even this could be lem-tribe-per-player, so that players aren't making crossover assignments to the same lems anyway.

From there, things will gradually build on what's been implemented at that point, and the need to allow same-frame-same-lem assignments may indeed present itself. But for now, there's no glaring need for it in single-player SuperLemmix, so it's probably not worth the replay complexity at this point.

Different-lem-same-frame, on the other hand, is a no-brainer. Investigating this now...

[WillLem]

I think this might be the relevant part of the Replay code that deals with deleting previous assignments:

Code: [Select]

procedure TReplay.Cut(aLastFrame: Integer; aExpectedSpawnInterval: Integer);
var
  NextSI: TReplayChangeSpawnInterval;

  procedure DoCut(aList: TReplayItemList; aLastFrameLocal: Integer);
  var
    i: Integer;
  begin
    for i := aList.Count-1 downto 0 do
      if aList[i].Frame >= aLastFrameLocal then aList.Delete(i);
  end;
begin
  DoCut(fAssignments, aLastFrame);

  NextSI := TReplayChangeSpawnInterval(SpawnIntervalChange[aLastFrame, 0]);
  if (NextSI <> nil) and (NextSI.NewSpawnInterval <> aExpectedSpawnInterval) then
    DoCut(fSpawnIntervalChanges, aLastFrame)
  else
    DoCut(fSpawnIntervalChanges, aLastFrame + 1);

  if fExpectedCompletionIteration > aLastFrame then
    fExpectedCompletionIteration := 0;

  fIsModified := true;
end;

Problem is, it doesn't seem to get called anywhere for deleting assignments, only for deleting SI changes...