Jibble

Author Topic: Error in the Default template's save system  (Read 1866 times)

Error in the Default template's save system
« on: 15 Jan 2016, 23:49 »
My attention was brought to this when I was testing someones game and found bizzare savesystem behavior. I cannot remember if I've seen this before, but assuming a lot of people around use Default template for their games, I believe this situation relates to many games, just had not been noticed or covered yet (?).

Okay, so here is how Default template saves a game when user already typed saves name and clicks on Save button:
Code: Adventure Game Studio
  1. ////////////////////////////////////////////////////////////////
  2.   int gameSlotToSaveInto = lstSaveGamesList.ItemCount + 1;
  3.   int i = 0;
  4.   while (i < lstSaveGamesList.ItemCount)
  5.   {
  6.     if (lstSaveGamesList.Items[i] == txtNewSaveName.Text)
  7.     {
  8.       gameSlotToSaveInto = lstSaveGamesList.SaveGameSlots[i];
  9.     }
  10.     i++;
  11.   }
  12.   SaveGameSlot(gameSlotToSaveInto, txtNewSaveName.Text);
  13. ////////////////////////////////////////////////////////////////
  14.  
(I removed some lines that are not important.)

What we have here is an algorithm of finding a slot to save the game into. How does default template do that?
It searches through all saves it found in the directory, until the name of the save matches with what user typed in the text box, or until it hits the end.
In the first case it saves game in the slot number, equal to the slot number with matching name, and in the second case it saves to the slot number equals to the total number of files in the list + 1.

Now, consider two test cases.

Case 1: we have several saves with properly ordered slot numbers:
Quote
agssave.001 -- named "Save 1"
agssave.002 -- named "Save 2"
agssave.003 -- named "Save 3"
agssave.004 -- named "Save 4"

User types in "Save 3" and game finds it in the slot 3. The game gets properly written into agssave.003 again.
User types in "New Save", games cannot find such slot and assigns slot number to the number of saves + 1, that is 5. The new game gets properly written into agssave.005.

Case 2: we have some save numbers missing in the beginning. How did this happen? Well, for instance, we were copying saves to other computer and missed some. Or maybe person uploaded single save to demonstrate a bug in the game, or something like this.
Quote
agssave.003 -- named "Save 3"
agssave.004 -- named "Save 4"

User types in "Save 3" and game finds it in the slot 3. The game gets properly written into agssave.003 again.
User types in "New Save", game cannot find such slot and assigns slot number to the number of saves + 1, that is... 3.
And the game is written to the existing file named agssave.003, overwriting game which we did not want to overwrite.
This also means, that player will now have a very low limit of save slots.
« Last Edit: 16 Jan 2016, 15:48 by Crimson Wizard »

Khris

  • having to deal with what games are going through
    • Lifetime Achievement Award Winner
    • I can help with play testing
    • I can help with scripting
    • I can help with translating
    • Khris worked on a game that was nominated for an AGS Award!
Re: Insane save system of the Default template
« Reply #1 on: 16 Jan 2016, 13:45 »
Regarding case 2, I can confirm the bug of overwriting an existing slot with a new name.

It does properly overwrite existing saves though, since it uses lstSaveGamesList.SaveGameSlots[i], not i as save slot.

Re: Insane save system of the Default template
« Reply #2 on: 16 Jan 2016, 15:42 »
It does properly overwrite existing saves though, since it uses lstSaveGamesList.SaveGameSlots[i], not i as save slot.
Right, I incorrectly diagnosed a cause of some other effect. I will fix the post.
« Last Edit: 16 Jan 2016, 15:46 by Crimson Wizard »

Re: Error in the Default template's save system
« Reply #3 on: 21 Jan 2016, 17:19 »
What about changing the script to following:

Code: Adventure Game Studio
  1. int find_save_slot(String name)
  2. {
  3.   bool slots[999];
  4.   int i = 0;
  5.   while (i < lstSaveGamesList.ItemCount)
  6.   {
  7.     if (lstSaveGamesList.Items[i] == txtNewSaveName.Text)
  8.     {
  9.       // found existing save with matching name
  10.       return lstSaveGamesList.SaveGameSlots[i];
  11.     }
  12.     // remember which slots are already taken
  13.     slots[lstSaveGamesList.SaveGameSlots[i]] = true;
  14.     i++;
  15.   }
  16.  
  17.   // Find first free save slot, starting with slot 1
  18.   i = 1;
  19.   while (i < 999)
  20.   {
  21.     if (!slots[i])
  22.       return i;
  23.     i++;
  24.   }
  25.   // no free slots found
  26.   return -1;
  27. }
  28.  
  29. function btnSaveGame_OnClick(GUIControl *control, MouseButton button)
  30. {
  31.   int gameSlotToSaveInto = find_save_slot(txtNewSaveName.Text);
  32.   if (gameSlotToSaveInto < 0)
  33.   {
  34.     Display("No more free save slots!");
  35.     return;
  36.   }
  37.   SaveGameSlot(gameSlotToSaveInto, txtNewSaveName.Text);
  38.   close_save_game_dialog();
  39. }
  40.  
« Last Edit: 22 Jan 2016, 01:08 by Crimson Wizard »

Gurok

  • Rottwheelers
  • When life hands you lemons, combine them with the mop
    • I can help with AGS tutoring
    • Best Innovation Award Winner 2016, for improving and extending the AGS scripting language
    • I can help with proof reading
    • I can help with scripting
    • Gurok worked on a game that won an AGS Award!
    •  
    • Gurok worked on a game that was nominated for an AGS Award!
Re: Error in the Default template's save system
« Reply #4 on: 22 Jan 2016, 00:36 »
I would just obtain the maximum gameSlotToSaveInto inside the existing loop (untested).

Code: [Select]
////////////////////////////////////////////////////////////////
  int gameSlotToSaveInto = lstSaveGamesList.ItemCount + 1;
  int i = 0;
  while (i < lstSaveGamesList.ItemCount)
  {
    if (lstSaveGamesList.Items[i] == txtNewSaveName.Text)
    {
      gameSlotToSaveInto = lstSaveGamesList.SaveGameSlots[i];
      i = lstSaveGamesList.ItemCount; // break;
    }
    else
    {
      if(lstSaveGamesList.SaveGameSlots[i] >= gameSlotToSaveInto)
      {
        gameSlotToSaveInto = lstSaveGamesList.SaveGameSlots[i] + 1;
      }
      i++;
    }
  }
  SaveGameSlot(gameSlotToSaveInto, txtNewSaveName.Text);
////////////////////////////////////////////////////////////////
« Last Edit: 22 Jan 2016, 00:38 by Gurok »

Re: Error in the Default template's save system
« Reply #5 on: 22 Jan 2016, 01:01 »
I would just obtain the maximum gameSlotToSaveInto inside the existing loop (untested).
I had that idea first, but what if there is only agssave998.sav lying in folder? No more saves.

BTW, thinking about it, my script is faulty, I will fix it.
« Last Edit: 22 Jan 2016, 01:07 by Crimson Wizard »

Gurok

  • Rottwheelers
  • When life hands you lemons, combine them with the mop
    • I can help with AGS tutoring
    • Best Innovation Award Winner 2016, for improving and extending the AGS scripting language
    • I can help with proof reading
    • I can help with scripting
    • Gurok worked on a game that won an AGS Award!
    •  
    • Gurok worked on a game that was nominated for an AGS Award!
Re: Error in the Default template's save system
« Reply #6 on: 22 Jan 2016, 01:18 »
I would just obtain the maximum gameSlotToSaveInto inside the existing loop (untested).
I had that idea first, but what if there is only agssave998.sav lying in folder? No more saves.

BTW, thinking about it, my script is faulty, I will fix it.

Ah yes, fair enough. It does have shortcomings. Best to stick with the array then.

Re: Error in the Default template's save system
« Reply #7 on: 23 Mar 2016, 16:20 »
Ugm... this was rather lucky that 999 bools accurately fit into the memory that AGS Script's allocates for arrays on stack. Which means that if this function (find_save_slot) is called from another function which has some more arrays allocated locally, game will stop with "stack overflow" error.

The array should probably be dynamic, or global. Latter will be slightly faster (it is not easy to predict this when dealing with AGS script interpreter) but have 4 kb of memory constantly allocated. ...I guess either is not a big deal anyway.
« Last Edit: 23 Mar 2016, 16:33 by Crimson Wizard »