Show Posts

This section allows you to view all posts made by this member. Note that you can only see posts made in areas you currently have access to.


Messages - Simon

Pages: 1 2 3 [4] 5 6 ... 279
46
I tested the Exp 6-A. Everything looks good:
  • Default option out-of-box is exit if save requirement met.
  • All three choices of the option behave during play as they should.
  • Text file contains one line only, which loads back as it should.
  • Mass replay verification produces byte-for-byte-identical files like the Exp 6, at least for Lemmings United and PimoLems. Haven't re-tested the others yet with the Exp 6-A.
In the end, it's up to you how much of today's feedback (my 2 earlier posts today) you still want to implement.

I can't estimate, e.g., how hard it is in the NL source (mostly in the code for the options dialog) to turn the 3 bools into an enum. Maybe it's sensible to address only the worry ("One way to avoid that worry is to remove the if/else entirely and write:").

If you touch the source at all before making the PR, then I'd rename the text-file option from "ExitToPostviewOption" to "ExitToPostview". It's easy now to rename it (there are 4 occurrences in the code), it'll be harder after release when people have the old string in their files.

Remember to rebase/to add the recent commits to the ExitToPostview upstream branch when you make the PR. They're still sitting on top of the Graphics32 update, but that was fine for me today to review.

Happy to schedule the evening on Monday, 13th for a review session in Mumble. Anything from Monday, 16:00 UTC through Monday, 22:00 UTC I can make.

-- Simon

47
You still have 3 internal variables, which can be a similar source of bugs as the 3 exposed options from the text file.

For example, here:

Code: [Select]
if not MatchStr(sOption, sPossibleOptions) then
    EndifLevelPassed := True
else begin
    AlwaysEndGameplay := (sOption = 'Always');
    EndIfLevelPassed := (sOption = 'IfLevelPassed');
    NeverEndGameplay := (sOption = 'Never');
end;

... you risk a bug whenever this runs while ((AlwaysEndGameplay is true) or (NeverEndGameplay is true)) and you run into the 'if'. I don't know if the code ever runs in that case. I know that it parses options during startup. But if it does run at another time, you'd again violate the invariant that exactly one of the three is true. This is a maintenance trap: The option-loading code now has a silent requirement that it may only be called once.

One way to avoid that worry is to remove the if/else entirely and write:

Code: [Select]
AlwaysEndGameplay := (sOption = 'Always');
NeverEndGameplay := (sOption = 'Never');
EndIfLevelPassed := not AlwaysEndGameplay and not NeverEndGameplay;

But even better: Track the option internally with one variable. Then it's absolutely impossible to have more than one of them true, it'll be obvious anywhere in the program and to any maintainer. Consider an enumeration; Delphi 6 should support them:

Code: [Select]
type TExitToPostview = (
    etpAlways,
    etpIfPassed,
    etpNever
);
var ExitToPostview : TExitToPostview;
Code: [Select]
if sOption = 'Always' then
    ExitToPostview := etpAlways;
else if sOption = 'Never' then
    ExitToPostview := etpNever;
else
    ExitToPostview := etpIfPassed;

I'll sit in Mumble for the night from 19:00 UTC to around 22:00 UTC. Catch me in Mumble spontaneously; otherwise, we'll review next week.

-- Simon

48
"End if Level Passed" should be the default

3 possible strings, one for each option ['Always', 'IfLevelPassed', 'Never'].
blank or nonsensical), we simply load the default.

multiple lines [...] only the first one is loaded & handled

Yes, all of this sounds great. Thanks!

Quote
MRC is behaving as we'd like (apart from the bug you reported here

I agree, it all feels correct.

I agree, we shouldn't worry about that bug (= talismans are not reported if we verify the replays under another username) because it sounds unrelated to our exit-to-postview work. I've already worked around it by changing my username.

Quote
The option is now stored as "ExitToPostviewOption"

With text strings as values (they are better than my earlier-suggested 0, 1, 2), I'd now name the option in the text file "ExitToPostview" (without the redundant "Option" at its end; everything in the file is an option). This is not a big worry.

Quote
Here's V6-A which implements these fixes (to Commit a2610dc).
ready for the PR now. Do let me know if there's any other last-minute tweaks

I can spare 2-3 hours tonight for testing; I'll post findings before UTC midnight.

After that, I'll be busy the next 4 days.

-- Simon

49
NL 12.12.5.

1. Build a level pack including some talisman levels.
2. In NL's options, name yourself X.
3. Cover your levelpack including all talismans with replays played by X.
4. As X, mass-verify your replay collection.
5. In NL's options, name yourself Y.
6. As Y, mass-verify your replay collection.

Expected: Identical output in steps 4 and 6 for each replay.
Observed: Different output in steps 4 and 6 for each talisman-winning replay.

When Y mass-verifies replays from X, then NL will still test for pass/fail correctly. NL will print pass/fail correctly to the screen and to the results file (the text output file in your NL worktree). But NL will refuse to tell Y whether X's replay achieved a talisman. NL will print talisman-winning replays as plain wins.

I understand that Y doesn't want X's talismans recorded in Y's personal statistics, but Y doesn't want X's plain wins written there, either. Then why does NL still print win/loss, but not talisman, during Y's mass-replay verification of X's replays?

Experienced NL designers/level maintainers: When you co-author a pack, or when you inherit a pack with talismans from another maintainer, how do you work with his replay collection? Do you mass-rename the player to yourself in the covering replays before you mass-verify the replays? Especially co-authoring a pack sounds impossible under this NL behavior without constantly renaming the players in all those replays.

-- Simon

50
In Exp 6 the following are all oneframediff (= the good, expected result):
  • All replays for Lemmings United non-bonus ranks,
  • all replays for NeoLemmix Introduction Pack,
  • all replays for PimoLems except for the a single replay for Straight Forward (in the rank called "One", level 7).
  • the 3 of your 5 replays from reply #102 ("Replay Check") that passed or won talismans.
All of these replays (including Straight Forward) test identical (down to the frame) in Exp 6 as they tested in 4 pm. These two NL versions generate byte-for-byte identical verification summary files.

Your undetermined replay from reply #102 is still undetermined, with the same number of physics updates, not oneframediff. That's probably fine? Your level-not-found replay from reply #102 still finds no level, good.

The only eyebrow-raiser is Straight Forward. This is a one-minute level in PimoLems. The replay is attached: This replay wins, then has only one blocker left standing that is never nuked, then runs over the time limit. WillLem, do you think that it's plausible that the NL replay verification takes the same number of physics updates, 1021 updates, in both NL 12.12.5 and Exp 6? We have:

1021 physics updates / 17 physics updates per second
= 60.0588 seconds
= 1 minute + 1/17 seconds
= 1 minute + 1 extra physics update

This sounds reasonable for a one-minute level. I think there is no need to worry here, especially since your undetermined replay from #102 also ran for identical physics update numbers in 12.12.5 and in Exp 6.

-- Simon

Edit 23:10 UTC: Added results for the 5 replays from WillLem's reply #102, a.k.a., "Replay Check" replays.

51
Lix Levels / Re: geoo's Lix level pack
« on: May 06, 2024, 05:08:20 PM »
Got it! ;P

Nice! Just Mine becomes easier when you solve the wrappy rank in order. But I told you to look directly at Just Mine (and skip the preceding 5 wrappy levels), thus, well done!

-- Simon

52
A few bugs in the options management, and I shed light on the lingering mismatching config (where we see X in the config menu but the game does Y). We'll start with the easiest.



Wrong default out of box.

1. Freshly extract Exp 6 from your full release.
2. Run NL.
3. Play a level (e.g., the included Mazulems level 1).
4. Fast-forward into death.

Expected: We get the panel message to rewind or quit.
Observed: We exit to postview.

IIRC, we agreed that the default should be to exit if we have won, and show the panel message when we lose. This is the most appropriate default. We had newbies (Darkshoxx in one of his streams) shout in surprise when the level exited on losing.



File contains three options, not one

1. Freshly extract Exp 6 from your full release.
2. Run NL.
3. Open the options menu (click on the cog in the main menu).
4. Press OK to save options.
5. Exit NL.
6. Open in a text editor: ./settings/settings.ini

Expected: One of the lines is for the end-of-level behavior.
Observed: Three of the lines are for the end-of-level behavior, e.g.:

AlwaysEndGameplay=1
EndIfLevelPassed=0
NeverEndGameplay=0


This invites to put nonsensical values in the file, e.g., set them all to 0, or set several of them to 1.

I recommend to store one line, e.g., with values 0, 1, 2. Call it, e.g., ExitToPostviewBehavior, or, if you'd like 0 to mean always-exit and want to keep that nuance in the text file, call it, e.g., StayInGameAfterLemmingsDied, although that's contrived.

Certainly, the user can still put nonsense values even when we use only a single line. E.g., he can text-edit the line to put negative values, or 3 or greater. It's probably okay to do whatever you want then, as long as it means the same as exactly one out of 0, 1, or 2. Maybe fix it under the hood, e.g., convert it to 0, 1, or 3 at options-loading time?

In particular, avoid the following mismatching bug for such nonsense inputs. It's the uncanny bug from months ago:



Mismatch from more than one 1

1. Produce a settings file with the Exp 6. (E.g., do all steps (1-6) from previous bug: File contains three option lines for the end-of-level behavior.)
2. Text-edit the file: Make all three lines end in 1 (none in 0).
3. Run NL.
4. Play a level and lose, e.g., kill all lemmings.
5. You automatically exit to postview.
6. Return to the NL main menu.
7. Open the options menu and go to the interface tab.

Expected: "Always Exit to Postview" is selected, matching our step 5.
Observed: "Never Exit to Postview" is selected.

Certainly, the user had to do something nonsensical to get here, and you can probably treat the garbage as what you like. But be consistent across game and options dialog.

-- Simon

53
Lix Main / Re: Error logging on Windows
« on: May 05, 2024, 09:11:38 PM »
I'm asking on the D forums: Does the D runtime support generating an error box for uncaught exceptions?

I don't believe it does. We'll generate the message box by hand.

Code: [Select]
version (Windows) {
    import core.sys.windows.windows;
    const wstring messageBody = /* ... */
    MessageBoxW(null, messageBody.ptr, null, MB_ICONERROR);
}

I'll put this in the catch-all that logs uncaught exceptions, then terminates Lix. Now, every time we log an uncaught exception on Windows, we'll also display an error box.

The box doesn't know about the Allegro 5 window (= Lix's main window). You can bring the hanging Lix window into the foreground, obscuring the modal box. Possible future investigation: Pass the Allegro 5 window instead of null for the first argument to MessageBoxW() and look at how the box interacts with the Lix window.

-- Simon

54
All right, 17:00 UTC on Saturday is it!

-- Simon

55
As far as I can tell, yes. We first check for Success, then check for a Talisman, wherever it's relevant. So, if we're checking for a Talisman then we have to have reached a Success result.

That doesn't answer my worry.

Reason: You're not exiting the monster, you're continuing into tests that care for (not success), but not for (not talisman) and overwrite the result that depended on your test for success:

Code: [Select]
... things that depend on success but don't exit early ...
if (Game.StateIsUnplayable or Game.IsOutOfTime)
    and not Game.GameResultRec.gSuccess then
        fReplays[i].ReplayResult := CR_FAIL;

This is a problem when the supplier of that result struct GameResultRec allows to have (gSuccess false at the same time as g.GotTalisman true). When I wrote "imply", I meant this supplier's side of concerns, not the consuming code that you showed. Your code seems to assume that the supplier always has gSuccess whenever he has g.GotTalisman. I ask you if that assumption is justified.

Your unassuming line would be:

Code: [Select]
and not (Game.GameResultRec.gSuccess or Game.GameResultRec.gGotTalisman) then
I'll have time for code review tonight, Friday 3rd, 17:00 UTC, or anytime on the weekend.

-- Simon

56
Lix Main / Re: Error logging on Windows
« on: May 01, 2024, 09:15:58 PM »
We link both the 32-bit and the 64-bit executables with lld-link. These executables will try to write all exceptions flying out of main() to stderr, which is closed because we suppress the console. Therefore, we see Lix terminate seemingly without error.

When we link a test project with Optlink (instead of with lld-link) and again suppress the console, we get the desired error box for exceptions flying out of main().

Is it possible to route the uncaught exceptions to error boxes even when we link with lld-link? I've looked through lld-link's command-line switches, and none of them look related. I'll ask on the D forums these days.

-- Simon

57
Thanks. Both packs that I tested yesterday (the NL Introduction pack and Lemmings United) produce identical text outputs during mass-replay-verification in your new nl-4pm as they produced in nl-2024-04-08. In other words, in nl-4pm, the replays are still oneframediff, which is good.

Your code (that you posted with nl-4pm) looks reasonable. I've only looked at your snippet, I haven't looked at the codebase. In those GameResultRec that you get there, does gGotTalisman always imply gSuccess? Then I don't see any problems.

I haven't looked at your test cases yet. The idea is excellent. I wouldn't worry much about the "Error" high-level result.

-- Simon

58
I call a replay oneframediff if and only if both of the following hold:
  • During mass-replay verification in NL 12.12.5, it shows the same high-level result (pass, fail, undetermined, ...) as during mass-replay verification in nl-2024-04-08.
  • Mass-replay verification in NL 12.12.5 takes exactly one physics update longer to run this replay than the mass-replay verification in nl-2024-04-08.
See my earlier long post for why I expect oneframediff replays.

Now:
  • All 208 replays in Lemmings United (excluding Bonus rank) are oneframediff.
  • All 120 replays in Lemmings Introduction pack are oneframediff.
  • I have 3 more Icho replay collections that I'll test for oneframediff-ness. Expect more results in the next 0-3 days.
Thus: Excellent so far!

Here is a Bash script that takes two NL output files (pass them as argument 1 and argument 2 to this script) and tells me if the replays are oneframediff:

#!bin/bash
paste "$1" "$2" \
    | sed 's%[^(]* (%,%' \
    | sed 's% [^(]*(%,%' \
    | sed 's% .*$%%' \
    | grep ^, \
    | grep -v "none" \
    | grep -v "TALISMAN" \
    | awk 'BEGIN { FS="," } { print $2 - $3; }'


The script prints the difference (see item 2 in my first enumerated list in this post) in run frames of the replay between the two NL versions. For a oneframediff replay, the script will print 1. If everything is oneframediff, you get a long stream of 1s.

The NL replay results format isn't the nicest to parse. But this script isn't particularly smart either; I should have relied on the consistently-appearing "... frames" in the NL replay format. Instead, the script cuts at open parentheses (, which will occiasionally appear in the level names. The script won't halt on such extra parentheses, but will produce garbage results over 1,000 or under −1,000 that we must examine by hand. All were fine.

-- Simon

59
SuperLemmix Bugs & Suggestions / Re: [SUG] New Lemming type - Rivals!
« on: April 30, 2024, 08:34:07 PM »
1) How should Rival exits be marked?

Avoid all possible mismatch, as mobius recommends.

If tribe alignment is a property of the exit: The exit is now responsible for showing tribe alignment, all by itself. E.g., add special cases near the code that draws exits during play.

It's possible to solve this by introducing extra tiles conditionally when the game constructs the level from a level file. But this feels unnatural. If you solve it with extra tiles, don't allow another way of adding these tiles; the only way must then be by setting exits' tribe alignment.

If tribe alignment is a result of overlaying two tiles: The overlapping tile is now responsible for showing alignment.

Quote
the idea here is that markers will then only be useful for the level designer whilst in the Editor, and the player whilst in the Player.

Assuming you make tribe alignment a property of the exit (and not a result of overlaying two tiles): Avoid reliance on human-placable marker tiles in the editor. The editor should render the exit as it will appear during play, or show other equivalent information on the exit.

Downside of introducing extra tiles conditionally when you render the map: The editor can't naturally show these tiles and not put them into the level.

Quote
At present, the plan is to not allow a lemming to be both a Neutral and a Rival.

Do you have deneutralizers? What happens when you neutralize a rival, then deneutralize him: Will he be regular lemming or rival?

-- Simon

60
Lix Main / Re: Error logging on Windows
« on: April 27, 2024, 11:20:02 AM »
Right.

First, #488: Logging (user/log.txt): Exceptions during init aren't logged. This will be easy to fix. I'll fix this in the next release, Lix 0.10.23. I've already put the fix in my unstable repo's master.

Second, on Windows, years ago, the D runtime used to generate a dialog box for unrecoverable errors (exceptions/errors flying out of main). Only after clicking away that dialog box, Lix would exit. Nowadays, we don't get this dialog box anymore. I don't know why we don't get the dialog anymore.

Third, on Windows, we suppress the console. This is correct. We'll ask people to look in user/log.txt for errors. The above #488 has prevented this during initialization, it won't prevent it from Lix 0.10.23 on.



We can investigate why there is no dialog box (above "Second, on Windows ..."). Possible relation with suppressing the console? Although we have suppressed the console for years, too.

-- Simon

Pages: 1 2 3 [4] 5 6 ... 279