Unable to save games in some (all?) engines

[criezy]

A recent regression prevent saving in all the engines I tried.

There are random assert that can happen when saving:

Assertion failed: (idx < _size), function operator[], file ./common/array.h, line 244

I got this one in at least BASS and Full Throttle.

Also even when the assert does not happen, in at least the SCUMM engine, saving does nothing (no save game is created). I reproduced this with DOTT and Full Throttle.

I tried a version from 8 days ago (commit 60111294a044fc) and that one is working properly.

I am not sure the two issues (assert, and no save file created) are the same issue, but it seems unlikely that two recent major regressions on saving games would not be at least connected.

[criezy]

I bisected the issue that no save is created in Full Throttle.
This is caused by

commit a17816f14dc8aece4bd88c411382107467dbb3b4
Author: Orgad Shaneh <orgads@gmail.com>
Date:   Tue Aug 24 23:02:42 2021 +0300

    GUI: Warn when saving a "young" game over an "older" one

[sev-]

This is obviously release critical.

[criezy]

I now found how to get the assert: I need to double click on the list rather than use the Enter key. So this will usually happen when overwriting an existing savegame or used the Autosave slot (i.e. a slot that already has some text). When typing the name it is more natural to then use the Enter key than a double click.

[criezy]

The issue about saving doing nothing is related to the addition of those two lines:

gui/saveload-dialog.cpp

@@ -579 +591 @@
 
        if (selItem >= 0 && _metaInfoSupport) {
                SaveStateDescriptor desc = (_saveList[selItem].getLocked() ? _saveList[selItem] : _metaEngine->querySaveMetaInfos(_target.c_str(), _saveList[selItem].getSaveSlot()));
+               if (!_saveList[selItem].getLocked())
+                       _saveList[selItem] = desc;
 
                isDeletable = desc.getDeletableFlag() && _delSupport;
                isWriteProtected = desc.getWriteProtectedFlag() ||

Removing those fixes that issue.

The assert is a separate issue that I am now bisecting.

[criezy]

The assert is a regression from

commit 44e5d3f9bdb64e000f97f234e96f85637c3bc1b0
Author: Orgad Shaneh <orgads@gmail.com>
Date:   Tue Aug 24 22:37:29 2021 +0300

    GUI: Factor out save/load activation to a function

[criezy]

The backtrace for the assert was:

* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
    frame #4: 0x00000001096b93c0 scummvm`Common::Array<Common::U32String>::operator[](this=0x000000011fc50640, idx=4294967295) const at array.h:244:3
   241 	
   242 		/** Return a const reference to the element at the given position in the array. */
   243 		const T &operator[](size_type idx) const {
-> 244 			assert(idx < _size);
   245 			return _storage[idx];
   246 		}
   247 	
Target 0: (scummvm) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
  * frame #4: Common::Array<Common::U32String>::operator[](this, idx=4294967295) const at array.h:244:3
    frame #5: GUI::ListWidget::getSelectedString(this) const at list.h:109:55
    frame #6: GUI::SaveLoadChooserSimple::handleCommand(this, sender, cmd, data=15) at saveload-dialog.cpp:469:27
    frame #7: GUI::CommandSender::sendCommand(this, cmd, data=15) at object.h:55:13
    frame #8: GUI::ListWidget::handleMouseUp(this, x=45, y=81, button=1, clickCount=2) at list.cpp:269:3
    frame #9: GUI::Dialog::handleMouseUp(this, x=53, y=109, button=1, clickCount=2) at dialog.cpp:230:6
    frame #10: GUI::GuiManager::processEvent(this, event, activeDialog) at gui-manager.cpp:694:17
    frame #11: GUI::GuiManager::runLoop(this) at gui-manager.cpp:430:4
    frame #12: GUI::Dialog::runModal(this) at dialog.cpp:79:8

The issue is that the ListWidget selected item is -1 (which becomes 4294967295 when casted to a `size_type).

[criezy]

I have now fixed both regressions, as well as a third one, all in the saveloy dialog.

5867d5420f GUI: Fix clicking in an empty space in the Grid Load dialog
626d0ed4b6 GUI: Fix assert when double clicking on item in list save dialog
0425dff824 GUI: Fix saving in a new slot with the list save dialog

Hopefully these were the only regressions introduced with the recent changes to the dialog and save code 🤞