retroarch: a save turned away gives back the slot it advanced

With Increment Save State Index on, Save State moves the slot up
before the save is pushed. A save the queue turned away - the saves
and loads held behind the one in flight already at their bound -
wrote nothing there, yet the slot stayed advanced, so the next Load
State named a slot with no file in it. command_event_main_state()
also called that save a success, and pruned old saves for it.

It returns false for a save turned away now, and prunes nothing for
it, and the slot goes back to where it was. A save that is pushed and
fails later on (a full disk) still leaves the slot advanced: later
saves may have moved it again by then.

The state-hold-full lane gains the case: with the hold full and
Increment Save State Index on, Save State must be refused and leave
the slot where it was. It fails on the old code.

command.c

@@ -7402,10 +7402,11 @@ bool command_event_main_state(unsigned cmd)
                      settings->uints.savestate_max_keep;
                bool frame_time_counter_auto_reset             =
                      settings->bools.frame_time_counter_auto_reset;
+               bool queued                                    = true;
 
                if (cmd == CMD_EVENT_SAVE_STATE)
                {
-                  bool queued = content_save_state(state_path, true);
+                  queued = content_save_state(state_path, true);
                   RARCH_LOG("[State] Save dispatch for slot %d, path "
                         "\"%s\": content_save_state queued=%s "
                         "(auto_index=%s, max_keep=%u). NOTE: actual disk "
@@ -7426,14 +7427,18 @@ bool command_event_main_state(unsigned cmd)
                else
                   content_save_state_to_ram();
 
-               /* Clean up excess savestates if necessary */
-               if (savestate_auto_index && (savestate_max_keep > 0))
+               /* Clean up excess savestates if necessary: not for a
+                * save that was turned away, which wrote nothing */
+               if (     queued
+                     && savestate_auto_index && (savestate_max_keep > 0))
                   command_event_set_savestate_garbage_collect(settings);
 
                if (frame_time_counter_auto_reset)
                   video_st->frame_time_count = 0;
 
-               ret      = true;
+               /* false for a save turned away, so the caller can take
+                * back the slot it advanced for it */
+               ret      = queued;
             }
             break;
          case CMD_EVENT_LOAD_STATE:

retroarch.c

@@ -4450,9 +4450,10 @@ bool command_event(enum event_command cmd, void *data)
       case CMD_EVENT_SAVE_STATE:
       case CMD_EVENT_SAVE_STATE_TO_RAM:
          {
+            int  prev_slot = settings->ints.state_slot;
+            bool advanced  = false;
             if (settings->bools.savestate_auto_index)
             {
-               int prev_slot = settings->ints.state_slot;
                int next_slot = command_event_get_next_savestate_auto_index(
                      settings);
                RARCH_LOG("[State] auto-index advance: slot %d -> %d "
@@ -4461,10 +4462,18 @@ bool command_event(enum event_command cmd, void *data)
                      settings->uints.savestate_max_keep);
                configuration_set_int(settings, settings->ints.state_slot,
                   next_slot);
+               advanced = true;
+            }
+            if (!command_event_main_state(cmd))
+            {
+               /* Turned away, the save wrote nothing to the slot moved
+                * to: the next load would have found it empty */
+               if (advanced && cmd == CMD_EVENT_SAVE_STATE)
+                  configuration_set_int(settings, settings->ints.state_slot,
+                     prev_slot);
+               return false;
             }
          }
-         if (!command_event_main_state(cmd))
-            return false;
          break;
       case CMD_EVENT_SAVE_STATE_DECREMENT:
          {

samples/tasks/content_load/content_load_test.c

@@ -2713,6 +2713,23 @@ static void lane_state_hold_full(void)
          "%u of 6 loads behind a save were taken, not some", taken);
    CHECK(queued_message(msg_hash_to_str(MSG_FAILED_TO_LOAD_STATE)),
          "a load turned away said nothing on screen");
+
+   /* With Increment Save State Index on, the slot moves up before the
+    * save is pushed; a save turned away wrote nothing there, and left
+    * advanced the slot named one the next load found empty. */
+   {
+      bool auto_index = settings->bools.savestate_auto_index;
+      int  slot       = settings->ints.state_slot;
+      configuration_set_bool(settings,
+            settings->bools.savestate_auto_index, true);
+      CHECK(!command_event(CMD_EVENT_SAVE_STATE, NULL),
+            "a save past the hold was taken");
+      CHECK(settings->ints.state_slot == slot,
+            "a save turned away moved the slot from %d to %d",
+            slot, settings->ints.state_slot);
+      configuration_set_bool(settings,
+            settings->bools.savestate_auto_index, auto_index);
+   }
    CHECK(state_tasks_settle(), "the held loads did not finish");
 
    remove(path);