[BUG][SUPERLEMMINI] SL not saving player progress [FIXED in SLToo]

[WillLem]

2nd EDIT: This bug has now been fixed for SuperLemminiToo, but still appears in SuperLemmini.



EDIT: This issue appears to be affecting individual levels rather than whole packs. It can happen with both custom packs and those bundled with SL(Too).



I have tested this with the following custom packs:

Lemmings, But With Lemminas!
Revenge Of The Lemmings - the level "Lem Dunk" (other levels seem to work)

The bug is that when a level is played with "Unlock All Levels" enabled (either via the option in SLToo or via the 0xdeadbeef debug mode in vanilla SL), the completion status and player records are not displayed, despite the level being completed. If  play continues for several more levels, completion status and player records are saved for the subsequent levels, but never the initially loaded level.

For example:

If I load Lemmings, unlock all levels, and complete just Fun 8 by itself, no records will be shown and the level is not marked as (completed).

If I play Fun 8 - Fun 12, records will be shown for levels 9 - 12, and these will also be marked as (completed), but level 8 will remain blank.



NOTE: I'm using Java JDK 17

[WillLem]

Tried a clean install this morning and I'm getting the same problem. It appears to be individual levels, rather than packs, which are affected by the issue.

[WillLem]

It might be something to do with the player profile itself. I created a new profile called "Test" and solved the Fun rank of LBWL - all levels showed as completed, even after restarting SL2.

I then copied the completion data across to my "WillLem" profile, and opened SL2 with that profile loaded. Only 3 of the levels were showing as completed, despite having an exact copy of the data from the "Test" profile (which still shows all levels as completed).



So, I tried it the other way around (copying the contents of "WillLem.ini" into "Test.ini"), and the problem came back when the Test profile was loaded. I then restored Test.ini and the level completion data for LBWL came back. So, it would seem that the problem is with the data in my player profile.

What's more baffling though, is that Lana reported having this same issue. So maybe the root cause of the problem is something in SL.

I've attached both the player .ini files in case they're of any use in solving this issue.

[WillLem]

Anyone?

I don't know enough Java to troubleshoot this myself, but if anyone might know at least where to start with something like this, I'll do my best to figure out what's going wrong and how to fix it in the absence of someone a bit more devvy than me...

[Simon]

The smallest example that you have is: Blank slate, install Lemminas, play and win levels 1 through 8 inclusive, then observe the wrong record on the unplayed level 9. This is a good start.

Can you reduce the bug further: What is the minimum number of levels to play to trigger the bug? For such kind of bug, there is a fair chance that it already triggers after playing 1 or 2 levels. Which levels? Can you trigger the bug by playing levels in default packs?

How does Lemmini ID levels? By file name, by level title, by an integer like NL, by something else?

Yeah, sounds like bug in SL when Lana runs into it on vanilla SL. But since nobody has a hunch what or where it is in the source, the most promising strategy is further reduction of the steps to reproduce the bug.

Loose idea for the reduction: 500 % lemmings saved on level 9 is 100 lemmings on level 9. Did you save 100 lemmings on a map before this?

-- Simon

[WillLem]

Thanks for the reply, Simon. Here's what's happened during the most recent bug test:

(This can be considered the clearest description of this bug as it currently stands.)

Clean install of SuperLemmini(Too)

Clean install of Lemmings, But With Lemminas! (Custom Pack from download on LF)

Completed all of Fun rank; all levels show as (completed), as does the rank itself - all stats are correct, no bugs so far!

For the sake of testing, completed levels 28 and 29 of Mayhem from Lemmings (Default pack as bundled with SL)

Mayhem 28 does not show as completed, but Mayhem 29 does

Let's test another random level... Tricky 21 All The 6s  - does not show as completed

Can I get another default level to show as completed at all?:
~ Taxing 28 does not show as completed
~ Fun 1-5 all show as completed (these were played one after another)
~ Skipped ahead - Fun 8 does not show as completed
~ Fun 9, however, does show as completed

Let's go back to LBWL and play some levels at random
~ Tricky 9 All The 9s does not show as completed
~ Taxing 9 Mary Poppins' Land does not show as completed
~ Mayhem 9 Just A Minute does not show as completed

Now let's try closing and re-opening SL - all records show exactly as stated above

Playing any of the above levels a second time makes no difference (tested with Fun 8 from original Lemmings and Taxing 9 from LBWL)

Other relevant information:

SL identifies levels by name.ini, and groups them by group_X, in the format: group_X = name.ini,Y where X is (rank #-1) and Y is (level position #-1)

The information for the previous given example (where levels 1-8 were completed, but level 9 was also showing as completed when it hadn't been played yet) is unfortunately now lost (through manual deletion of the level completion data during bug testing) and cannot be reproduced

The bug doesn't seem to happen at all in original Lemmini (also, original Lemmini doesn't store level completion data in the same way), so this can most likely be attributed to SL(Too) code

There don't seem to be any levels in particular which trigger the bug, it seems to be random. This is verified by the fact that re-installations of SL and new player profiles can generate entirely different level completion data

If several levels are played one after another from the beginning of a pack/rank, they usually all show as completed (although this has not been the case during previous testing of this bug)

If several levels are played one after another from the middle of a pack/rank, they usually all show as completed except the first one played (although this has not been the case during previous testing of this bug)

The bug can happen for both default and custom packs

Levels from within the same rank/pack can generate different level completion data - so the bug isn't tied to rank or pack

The level completion data can be different from player to player, even in the same copy of SuperLemmini on the same machine (verified by playing the same levels anew on 2 different - and new - player profiles: each generated different level completion data)



Conclusion:

More data needed!

Is anyone else experiencing this bug? Does anyone have any idea what might be causing it?

[WillLem]

OK, I seem to have confirmed that it's possible to mark a level as completed by completing both the level before it and the current level.

So, levels are only marked as completed if played one after another except the first level played unless that level is at the beginning of the rank. Although, I have been able to mark some random levels as completed during previous testing, so this behaviour appears not to be consistent, unless my memory doesn't quite serve me correctly...

Anyway, my hunch is that it has something to do with the new "Unlock All Levels" feature in SuperLemmini(Too) - although this wouldn't necessarily explain why the bug also occurs in vanilla SuperLemmini...



EDIT: OK, I tried debug mode in vanilla SL (which unlocks all levels) and this bug is occurring there too. I'm pretty sure it has something to do with unlockAllLevels=true, since the only pack(s) which seem(s) to be affected in vanilla SuperLemmini are those which have all levels unlocked (which is all packs when in debug mode), and SLToo allows all levels to be unlocked as an option.

I THINK I'M GETTING SOMEWHERE!

[WillLem]

src > lemmini > game > Player.java contains the code regarding level completion.

{TotalGuesswork} UNLOCKALLLEVELS ignores whether or not the current level is or has been completed, and unlocks all levels after it, which is almost certainly what is causing this bug. {/TotalGuesswork}

As a player plays through a pack with UNLOCKALLLEVELS enabled, it only marks those as completed which are loaded after the initially selected level, but not the initially selected level itself. The only exception to this is the first level of each rank, which is always unlocked anyway and can therefore be subject to completion even with UNLOCKALLLEVELS enabled.

I've come this far, and I'm about 88% certain that I've found the source of the bug, but I still have no idea how to fix it. Any help/advice is very welcome at this point!

[WillLem]

Further investigation of this bug by DireKrow (thanks!) and myself has yielded the following:

The bug could possibly be resulting from the way that player records are displayed, as opposed to how they're saved. This is evident because the player.ini save file shows records for all levels that are completed, regardless of whether the completion data is being displayed in the menu.

Most of the code regarding records / level unlocking / cheat mode can be found in Player.java and GameController.java; (the former has code for this throughout the file, the latter specifically refers to record saving on line 2560). It's my hunch that Unlock All Levels and/or "cheat mode" (which is actually SL's debug mode) interferes somehow with the saving and/or displaying of records (at this point, it seems more likely to be the displaying).

Experiment:

The code on line 2560 of GameController.java basically says "if the level was not lost, and not cheated, then show the player records for this level. Otherwise, show a blank record."

I removed the part which says "and not cheated" to see what would happen.

SL did indeed begin to display records again upon playing and completing random levels throughout a fully-unlocked pack. However, somewhat bizarrely, it displayed the records for the subsequent level after the one which had been played. So, if I played Fun 8, it would mark Fun 9 as (completed) and display the records for Fun 9 (presumably from a previous playthrough of that level).

Can open. Worms everywhere. Done with it for tonight, I'll tidy up tomorrow

[Charles]

Fantastic debug work, WillLem!

The Player records wasn't something I'd ever looked at before, so SLToo didn't change any of the code there, but I certainly exacerbated this problem by more easily exposing the UnlockAllLevels stuff.

Reviewing it now, I can confirm that commas in the group names do break the records.  It's a case of Props.java not doing any special handling of commas when writing, and then just reading it in as if it was a standard csv (splitting into columns by the commas).

You've probably seen a line like this before in the player.ini files

Code Select Expand

group13=lemmings, but with lemminas\!-fun, 5
That's saving the record details in the form of group<IDX>=<NAME>-<RATING>, <UNLOCKED_BITS>
The <UNLOCKED_BITS> part is also interesting in itself.  It's a binary number representing which levels are unlocked. 1 means the level is unlocked, 0 is locked. i.e from the above, 5 in binary is 0101, so (starting at 0 and reading from right to left) levels 0 and 2 are unlocked, while all the others are locked.

I didn't play around with line 2560 in GamneController.java (my line 2560 didn't seem to line up with yours anyway), but I did play around with Player.java. I think I found some code that was promising, but I couldn't do much testing.

To help me learn, I was adding a bunch of comments and System.out.println() statements, so my line numbers won't match yours but, look for the part in the constructor that looks like this:

Code Select Expand


                for (int j = 0; j < unlockedLevels.bitLength(); j++) {
                    if (j == 0 || unlockedLevels.testBit(j)) {
                        boolean completed = props.getBoolean("group" + idx + "_level" + j + "_completed", false);
                        if (completed) {
                            int lemmingsSaved = props.getInt("group" + idx + "_level" + j + "_lemmingsSaved", -1);
                            int skillsUsed = props.getInt("group" + idx + "_level" + j + "_skillsUsed", -1);
                            int timeElapsed = props.getInt("group" + idx + "_level" + j + "_timeElapsed", -1);
                            int score = props.getInt("group" + idx + "_level" + j + "_score", -1);
                            levelRecords.put(j, new LevelRecord(completed, lemmingsSaved, skillsUsed, timeElapsed, score));
                        } else {
                            levelRecords.put(j, LevelRecord.BLANK_LEVEL_RECORD);
                        }
                    }
                }


(Just before this code the game has loaded the level group name/rating and the unlocked bits.) So what's going on here is that the game is now loading the individual level stats for that level group/rating. BUT first off, it's only checking up to the highest unlocked level (j < unlockedLevels.bitLength();), and then secondly if that level is flagged as not unlocked, then it's not even bothering to load that level's stats at all.

I haven't looked at the code that decides what levels are locked or unlocked, and probably this whole section will need an overhaul, but a quick fix might be to just comment out the if (j == 0 || unlockedLevels.testBit(j)) bit, so it loads all level stats regardless if the level's flagged as locked or not.

So...

Code Select Expand


                for (int j = 0; j < unlockedLevels.bitLength(); j++) {
                    //if (j == 0 || unlockedLevels.testBit(j)) {
                        boolean completed = props.getBoolean("group" + idx + "_level" + j + "_completed", false);
                        if (completed) {
                            int lemmingsSaved = props.getInt("group" + idx + "_level" + j + "_lemmingsSaved", -1);
                            int skillsUsed = props.getInt("group" + idx + "_level" + j + "_skillsUsed", -1);
                            int timeElapsed = props.getInt("group" + idx + "_level" + j + "_timeElapsed", -1);
                            int score = props.getInt("group" + idx + "_level" + j + "_score", -1);
                            levelRecords.put(j, new LevelRecord(completed, lemmingsSaved, skillsUsed, timeElapsed, score));
                        } else {
                            levelRecords.put(j, LevelRecord.BLANK_LEVEL_RECORD);
                        }
                    //}
                }

The only problem that still might remain is if there are stats for levels higher than the highest level flagged as unlocked.  For example, going back to my example with the 5 above, only levels 0 and 2 are flagged as unlocked, so it still won't load stats for levels 3-30 or higher. And that can't be fixed without a major change... the total number of levels in a level group/rating just isn't saved here at all.

[WillLem]

Thanks for the reply, Charles! It would be great to work with you on this one.

Reviewing it now, I can confirm that commas in the group names do break the records

Ah, OK. I feel like maybe this shouldn't be the case... is it worth adding some code for handling of special characters in group names, or is this a bigger task than I'm imagining it might be...?

a quick fix might be to just comment out the if (j == 0 || unlockedLevels.testBit(j)) bit, so it loads all level stats regardless if the level's flagged as locked or not.

Yes, I feel like SL should be doing this; it shouldn't matter whether a level is locked or not (if it's been completed, that should unlock it anyway).

The only problem that still might remain is if there are stats for levels higher than the highest level flagged as unlocked ... And that can't be fixed without a major change... the total number of levels in a level group/rating just isn't saved here at all.

Hmm. Could it be made so that any available stats are displayed, regardless of a level's locked/unlocked status? It seems to me that these factors ought to not be intertwined at all. So: if a level is locked/unlocked, this should only affect the list of levels being displayed. Stats for listed levels (whether locked or unlocked) should always be displayed... I wonder if uncoupling the code by simply removing it would be enough to achieve this, or whether there are other lines of code elsewhere which depend on this relationship.

I tried this:

Code Select Expand

BigInteger unlockedLevels = ToolBox.parseBigInteger(s[1]);
                Map<Integer, LevelRecord> levelRecords = new LinkedHashMap<>();
                for (int j = 0; j < unlockedLevels.bitLength(); j++) {
                    //if (j == 0 || unlockedLevels.testBit(j)) {
                        boolean completed = props.getBoolean("group" + idx + "_level" + j + "_completed", false);
                        if (completed) {
                            int lemmingsSaved = props.getInt("group" + idx + "_level" + j + "_lemmingsSaved", -1);
                            int skillsUsed = props.getInt("group" + idx + "_level" + j + "_skillsUsed", -1);
                            int timeElapsed = props.getInt("group" + idx + "_level" + j + "_timeElapsed", -1);
                            int score = props.getInt("group" + idx + "_level" + j + "_score", -1);
                            levelRecords.put(j, new LevelRecord(completed, lemmingsSaved, skillsUsed, timeElapsed, score));
                        //} else {
                            //levelRecords.put(j, LevelRecord.BLANK_LEVEL_RECORD);
                        }
                    //}
                }

It sort of worked. Suddenly, loads of levels that were missing records had records showing. But, only up to a certain level (as you said would be the case). I figured that removing the "else = blank record" script altogether might leave it so that records would be displayed for all levels, whether they were unlocked or not. But, it seems that the  "up to a certain point" code is still being naughty.

I then tried removing this entire section of code altogether, and it wiped my records (luckily I had a backup!). Really not sure what to do about this, but willing to learn.

[Charles]

is it worth adding some code for handling of special characters in group names, or is this a bigger task than I'm imagining it might be...?

I'm playing with some code, which I think is a "good enough" solution. It's tricky, because the "proper" way to fix it would be to fix the code in Props.java, right at the source in getArray (actually there's a bunch of getArray functions, so you'd want to fix them all). The getArray functions are little helper functions that retrieve a value from an ini file, and then treat it like a CSV array (comma separated value). It splits the string into an array of smaller strings wherever it finds a comma, then trims any spaces at the beginning and end of each one. (That trimming is important to note for later)
So a more thorough fix would be to create a setArray function that accepts an array as a parameter and does proper character escaping when it encounters a comma (i.e. replace , with \, or something), then fix up the getArray to recognize those character escapings when reading it back in.

I didn't do that. I took a lazier approach:

Code Select Expand


            System.out.println("    loading level group " + idx);
            // first string is the level group key identifier
                // second string is a BigInteger used as bitfield to store available levels
                String[] s = props.getArray("group" + idx, null);
                if (s == null || s.length < 2 || s[0] == null) {
                    System.out.println("    no valid group data found... breaking...");
                break;
                }
                String s1 = s[s.length-1]; // get the last
                String groupName = "";
                // due to a bug in the ini property "Array" saving, commas are not escaped, and so they're treated as you would any column separator
                // because the getArray function above trims out any spaces, knowledge if there was or was not a space after a comma is lost
                // so let's just assume there's always a space after it, because that's better grammar.
                for (int a = 0; a < s.length-1; a++) {
                groupName += s[a];
                if (a < s.length-2 ) {
                groupName +=", "; //we're assuming there's always a space after any comma
                }
                }
                System.out.println("    group name identified: " + groupName);

So previously the code would getArray to get the CSV from the Player.ini file. It would only proceed if there were exactly 2 values: 1) the group name, and 2) the unlockedLevels. More or less than 2 values, and it would skip onto the next group.  So now, I read in all values, and only skip if there are less than 2. Instead of taking the 2nd value as the unlockedBits, I take the last value as the bits.
For the group name, I concatenate all the other values together. And I put a ", " between them.

So, the only thing I've lost is that because the getArray trims any spaces, I have no way of knowing if there's supposed to be a space after the comma or not. So I just assume there is, and leave it at that.

I figured that removing the "else = blank record" script altogether might leave it so that records would be displayed for all levels, whether they were unlocked or not. But, it seems that the  "up to a certain point" code is still being naughty.

I then tried removing this entire section of code altogether, and it wiped my records (luckily I had a backup!). Really not sure what to do about this, but willing to learn.

It's not surprising that commenting out that (or a part of it) didn't help. The else put(BLANK_LEVEL_RECORD) part isn't replacing or hiding existing records.

For each level, the game saves "completed (true/false)", "lemmingsSaved (integer)", "skillsUsed (integer)", "timeElapsed (integer)", "score (integer)".  The assumption is that if "completed" is false (or doesn't exist) then logically the other values shouldn't exist either... they're only stats for if the level is completed.  So, if the level's not completed, don't even try to load non-existant stats. Just create a blank record.  I think that assumption still holds water.

Could it be made so that any available stats are displayed, regardless of a level's locked/unlocked status?

Yes, but not without some modifications.  That's exactly what the code we commented out in the first place did.  if ( j == 0 || unlockedLevels.testBit(j) )  That's saying, try and load the records if it's the 1st level in the group, OR the level is unlocked.  Removing that just loads all the stats up to the last level the ini files knows about. Unfortunately that is the problem. The INI file doesn't know how many levels total there are... it can't tell the code where to stop, and at this point, the INI file is all we're asking.

So, I see 2 possible solutions around that: 1) add the total level count to the ini file, or 2) query whatever does know how many levels there are.

Stats for listed levels (whether locked or unlocked) should always be displayed...

That's also a valid 3rd option.  But I don't know that I'd consider any option trivial.

For option 3, the difficulty is that currently the code specifies what key to read by name, one after the other. i.e. group0_level0_completed, then group0_level1_completed, ... , group0_level99_completed, ... , group0_level100_completed... etc.  Without knowing how many levels there are, what number should you decide to stop at?  Instead you have to flip that on it's ear, and loop through all the properties that do exist. I don't believe that code exists yet, so you'd have to write it.  Probably best to put it in the Props.java. It might look something like:

Code Select Expand


public int getHighestLevel(int groupNum) {
    //psuedo-code
    for each key in Properties
    if key.startsWith("group" + grounNum + "_level") then
        get the digits up to the next _ and compare with the next key until you find the highest one.

   return highestNum;
}

Hmm... yeah, actually I do like option 3 the best. It retains the most backwards compatibility with SuperLemmini ... and, if there's no levels saved, who cares if it's unlocked or not (as far as record showing goes).

The other options I was thinking of require making changes to how the INI is saved, or I don't even know yet where to read in the total level number otherwise.

[Charles]

Sorry for the double-post, but I went ahead and wrote up option 3 afterall.

So, this part goes in your Props.java:

Code Select Expand


    /**
     * Returns the highest level number present for the supplied Group, in the records.
     * @param groupNum The group number from the player INI file to check levels against
     * @return -1 if no matches found, or the highest level number otherwise (starting at 0).
     */
    public int getHighestLevel(int groupNum) {
    Set<Object> keys = hash.keySet();
    int max = -1;
    for(Object k:keys) {
    String key = k.toString();
    String match = "group" + groupNum + "_level";
    if(key.startsWith(match)) {
    int idx2 = key.indexOf("_", match.length()-1);
    String tmpNum = key.substring(match.length(), idx2);
    try {
    int num = Integer.parseInt(tmpNum);
    if (num > max) {
    max = num;
    }
    } catch (NumberFormatException e) {
    //just catching errors because we have no way of trusting the data we're inputting.
    //but we don't need to actually do anything with the error... just to prevent it from bringing down the whole system.
    }
    }
    }
    return max;
    }


And in Player.java (in the constructor), replace this:

Code Select Expand


                for (int j = 0; j < unlockedLevels.bitLength(); j++) {
                    if (j == 0 || unlockedLevels.testBit(j)) {
                        boolean completed = props.getBoolean("group" + idx + "_level" + j + "_completed", false);
                        if (completed) {
                            int lemmingsSaved = props.getInt("group" + idx + "_level" + j + "_lemmingsSaved", -1);
                            int skillsUsed = props.getInt("group" + idx + "_level" + j + "_skillsUsed", -1);
                            int timeElapsed = props.getInt("group" + idx + "_level" + j + "_timeElapsed", -1);
                            int score = props.getInt("group" + idx + "_level" + j + "_score", -1);
                            levelRecords.put(j, new LevelRecord(completed, lemmingsSaved, skillsUsed, timeElapsed, score));
                        } else {
                            levelRecords.put(j, LevelRecord.BLANK_LEVEL_RECORD);
                        }
                    }
                }


with this:

Code Select Expand


                int maxLevel = props.getHighestLevel(idx) + 1;
                maxLevel = Math.max(maxLevel, unlockedLevels.bitLength());
                for (int j = 0; j < maxLevel; j++) {
                    //if (j == 0 || unlockedLevels.testBit(j)) {
                        boolean completed = props.getBoolean("group" + idx + "_level" + j + "_completed", false);
                        if (completed) {
                            int lemmingsSaved = props.getInt("group" + idx + "_level" + j + "_lemmingsSaved", -1);
                            int skillsUsed = props.getInt("group" + idx + "_level" + j + "_skillsUsed", -1);
                            int timeElapsed = props.getInt("group" + idx + "_level" + j + "_timeElapsed", -1);
                            int score = props.getInt("group" + idx + "_level" + j + "_score", -1);
                            levelRecords.put(j, new LevelRecord(completed, lemmingsSaved, skillsUsed, timeElapsed, score));
                        } else {
                            levelRecords.put(j, LevelRecord.BLANK_LEVEL_RECORD);
                        }
                    //}
                }


It's just the top three lines that are changed.  We'll get the highest level num for this group in the ini level records, and just in case if the unlocked bits is higher, we'll take that instead.

[WillLem]

Nice work, Charles!

I ran the code as suggested, and can confirm that it works for all levels played after the code is applied to SL. All existing records in player.ini, however, are not shown (but nor are they wiped from player.ini, as I accidentally did yesterday when trying a few things out!)

I'm assuming here that getHighestLevel is meant to be reading and displaying existing data from player.ini files, but if I'm wrong about that please let me know. I tried a couple of tweaks to see if I could get it to stop displaying blanks, but nothing worked.

To clarify, though, new data inputted after the code has been run is displayed as expected, so you're definitely on the right track with getHighestLevel. Maybe it needs to specifically look for old data as well, though...(thought: actually, yeah...where is it getting the data from, if not player.ini? And why, then, is it not displaying all of the data that it sees there )

[Charles]

There's no distinction between "old" data and "new" data in player.ini.

Maybe it's time to look at the record display code. I think it's loading all the records correctly this time. At least I can't think of any gaps in our loading code.