MCP: review fixes for frames, RAM, hardcore

cheevos/cheevos.c

@@ -849,6 +849,11 @@ bool rcheevos_hardcore_active(void)
    return rcheevos_locals.hardcore_being_enabled;
 }
 
+bool rcheevos_hardcore_in_game(void)
+{
+   return rcheevos_hardcore_active() && rcheevos_is_game_loaded();
+}
+
 void rcheevos_pause_hardcore(void)
 {
    rcheevos_locals.hardcore_allowed = false;

cheevos/cheevos.h

@@ -45,6 +45,8 @@ void rcheevos_pause_hardcore(void);
 void rcheevos_hardcore_enabled_changed(void);
 void rcheevos_toggle_hardcore_paused(void);
 bool rcheevos_hardcore_active(void);
+/* Hardcore mode in force: on, with a game whose achievements are loaded */
+bool rcheevos_hardcore_in_game(void);
 
 bool rcheevos_is_pause_allowed(void);
 void rcheevos_spectating_changed(void);

command.c

@@ -63,6 +63,7 @@
 #include "core_info.h"
 #include "playlist.h"
 #include <queues/task_queue.h>
+#include <retro_atomic.h>
 #include "cheat_manager.h"
 #include "content.h"
 #include "dynamic.h"
@@ -965,9 +966,10 @@ static bool command_screenshot(command_t *cmd)
    return true;
 }
 
+#ifdef HAVE_RPNG
 /* GET_FRAME [size]: the frame on screen as image content, scaled to fit
- * a square of @size and never up. Nothing is written, so it is cheap
- * enough to call while a queued STEPS plays. */
+ * a square of @size and never up. Nothing is written: a software core's
+ * frame is copied, a hardware core's read back from the GPU. */
 bool command_get_frame(command_t *cmd, const char *arg)
 {
    command_deferred_t *d;
@@ -1012,6 +1014,7 @@ bool command_get_frame(command_t *cmd, const char *arg)
    return true;
 }
 #endif
+#endif
 
 /* A command answered once the work it starts is through: by @ok, else
  * by @name and the task's result, or by @name, ERROR and the reason. */
@@ -3677,20 +3680,46 @@ void command_run_frames_free_frame(void)
 const char *command_tools_refused(void)
 {
 #ifdef HAVE_CHEEVOS
-   if (rcheevos_hardcore_active())
+   if (rcheevos_hardcore_in_game())
       return "achievements hardcore mode is on; the player can turn it off";
 #endif
    return NULL;
 }
 
+#ifdef HAVE_THREADS
+/* hardcore mode took effect on a task thread, as a game's achievements
+ * finish loading: what it stops waits for the main thread's next frame */
+static retro_atomic_int_t command_hardcore_due;
+#endif
+
 void command_hardcore_entered(void)
 {
+#ifdef HAVE_THREADS
+   if (!task_is_on_main_thread())
+   {
+      retro_atomic_store_release_int(&command_hardcore_due, 1);
+      return;
+   }
+#endif
+   /* a queued job keeps why it ended, told once hardcore mode is off */
    if (runloop_state_get_ptr()->frame_work & RUNLOOP_WORK_RUN_FRAMES)
       command_run_frames_cancel("achievements hardcore mode is on");
-   command_run_frames_forget();
+   else
+      command_run_frames_forget();
    input_driver_command_release(MAX_USERS);
 }
 
+void command_frame_start(void)
+{
+#ifdef HAVE_THREADS
+   if (retro_atomic_exchange_int(&command_hardcore_due, 0))
+      command_hardcore_entered();
+#endif
+#ifdef HAVE_MENU
+   command_menu_closed();
+#endif
+}
+
 void command_run_frames_forget(void)
 {
    if (command_run_frames_st.prev[0])
@@ -5799,33 +5828,44 @@ static const rarch_memory_descriptor_t* command_memory_get_descriptor(const rarc
    return NULL;
 }
 
-/* A core with no memory map still hands over its RAM as blocks: system
- * RAM from address 0, save RAM straight after it, as the achievements
- * lay them out when they know no console - so system RAM is at the
- * addresses RetroAchievements' notes give. @n is 0 for system RAM, 1 for
- * save RAM; NULL where the core has none. */
-static uint8_t *command_memory_core_block(unsigned n, size_t *start,
-      size_t *size)
+/* A core with no memory map still hands over its RAM as blocks, once a
+ * game is loaded: system RAM from address 0 and save RAM straight after
+ * it, as the achievements lay them out when they know no console - for
+ * the SNES, the addresses RetroAchievements' notes give. Fills @blocks
+ * (data, start, size) for the system and the save block, a NULL data
+ * where the core has none; false where it has neither. */
+struct command_ram_block
+{
+   uint8_t *data;
+   size_t   start;
+   size_t   size;
+};
+
+static bool command_memory_core_blocks(struct command_ram_block blocks[2])
 {
    runloop_state_t        *runloop_st = runloop_state_get_ptr();
-   retro_ctx_memory_info_t sys;
-   retro_ctx_memory_info_t save;
-   if (     !runloop_st->current_core.retro_get_memory_data
+   retro_ctx_memory_info_t info;
+   memset(blocks, 0, 2 * sizeof(*blocks));
+   if (     !(runloop_st->current_core.flags & RETRO_CORE_FLAG_GAME_LOADED)
+         || !runloop_st->current_core.retro_get_memory_data
          || !runloop_st->current_core.retro_get_memory_size)
-      return NULL;
-   sys.id = RETRO_MEMORY_SYSTEM_RAM;
-   core_get_memory(&sys);
-   if (!sys.data)
-      sys.size = 0;
-   *start = 0;
-   *size  = sys.size;
-   if (n == 0)
-      return sys.size ? (uint8_t*)sys.data : NULL;
-   save.id = RETRO_MEMORY_SAVE_RAM;
-   core_get_memory(&save);
-   *start = sys.size;
-   *size  = save.data ? save.size : 0;
-   return *size ? (uint8_t*)save.data : NULL;
+      return false;
+   info.id = RETRO_MEMORY_SYSTEM_RAM;
+   core_get_memory(&info);
+   if (info.data && info.size)
+   {
+      blocks[0].data = (uint8_t*)info.data;
+      blocks[0].size = info.size;
+   }
+   info.id = RETRO_MEMORY_SAVE_RAM;
+   core_get_memory(&info);
+   if (info.data && info.size)
+   {
+      blocks[1].data  = (uint8_t*)info.data;
+      blocks[1].start = blocks[0].size;
+      blocks[1].size  = info.size;
+   }
+   return blocks[0].data || blocks[1].data;
 }
 
 static uint8_t *command_memory_get_pointer(
@@ -5835,21 +5875,17 @@ static uint8_t *command_memory_get_pointer(
 {
    if (!sys_info || sys_info->mmaps.num_descriptors == 0)
    {
-      unsigned n;
-      bool     any = false;
+      struct command_ram_block blocks[2];
+      unsigned                 n;
+      bool                     any = command_memory_core_blocks(blocks);
       for (n = 0; n < 2; n++)
-      {
-         size_t   start = 0;
-         size_t   size  = 0;
-         uint8_t *block = command_memory_core_block(n, &start, &size);
-         if (block)
-            any = true;
-         if (block && address >= start && address < start + size)
+         if (     blocks[n].data && address >= blocks[n].start
+               && address < blocks[n].start + blocks[n].size)
          {
-            *max_bytes = (unsigned int)(start + size - address);
-            return block + (address - start);
+            *max_bytes = (unsigned int)(blocks[n].start + blocks[n].size
+                  - address);
+            return blocks[n].data + (address - blocks[n].start);
          }
-      }
       if (any)
          strlcpy_lit(s, " -1 no descriptor for address\n", len);
       else
@@ -5916,15 +5952,15 @@ bool command_get_memory_map(command_t *cmd, const char *arg)
     * it, in an address space named core */
    if (!mmap || !mmap->num_descriptors)
    {
+      struct command_ram_block blocks[2];
+      command_memory_core_blocks(blocks);
       for (i = 0; i < 2; i++)
       {
-         size_t start = 0;
-         size_t size  = 0;
-         char   s_start[24], s_len[24];
-         if (!command_memory_core_block(i, &start, &size))
+         char s_start[24], s_len[24];
+         if (!blocks[i].data)
             continue;
-         command_hex(s_start, start);
-         command_hex(s_len,   size);
+         command_hex(s_start, blocks[i].start);
+         command_hex(s_len,   blocks[i].size);
          _len += (size_t)snprintf(reply + _len,
                COMMAND_LIST_REPLY_MAX + 1 - _len,
                "%s\t%s\t0\t0\t%s\trw\tlittle\tcore\n", s_start, s_len,

command.h

@@ -616,9 +616,9 @@ struct cmd_action_map
    const struct cmd_rows *rows;
 };
 
-#ifdef HAVE_SCREENSHOTS
+#if defined(HAVE_SCREENSHOTS) && defined(HAVE_RPNG)
 /* GET_FRAME [size]: the frame on screen, as image content where the
- * interface carries one */
+ * interface carries one; a PNG, built in memory */
 #define COMMAND_FRAME_SIZE     320
 #define COMMAND_FRAME_SIZE_MIN 64
 #define COMMAND_FRAME_SIZE_MAX 640
@@ -660,6 +660,9 @@ void command_run_frames_forget(void);
  * a frame job, a queue, held input - as hardcore mode takes effect. */
 const char *command_tools_refused(void);
 void command_hardcore_entered(void);
+/* On the main thread each frame, before commands are read: what other
+ * threads left for it, and a MENU request whose menu closed. */
+void command_frame_start(void);
 void command_run_frames_free_frame(void);
 
 /* SET_INPUT <player> <control> <value> / RELEASE_INPUT [player], and
@@ -946,10 +949,10 @@ static const struct cmd_param cmd_params_playlist[] = {
       CMD_PARAM_INT, false, NULL, NULL, "0" },
    { NULL }
 };
-#ifdef HAVE_SCREENSHOTS
+#if defined(HAVE_SCREENSHOTS) && defined(HAVE_RPNG)
 static const struct cmd_param cmd_params_get_frame[] = {
    { "size", "The longest side at most, 64 to 640; 320 when left out.",
-      CMD_PARAM_INT, false, NULL, NULL, NULL },
+      CMD_PARAM_INT, false, NULL, NULL, "320" },
    { NULL }
 };
 #endif
@@ -1237,7 +1240,7 @@ static const struct cmd_action_map action_map[] = {
 #endif
    { "VERSION",          command_version,          "No argument", "Report the RetroArch version.", CMD_INFO_READ_ONLY | CMD_INFO_NO_TOOL, NULL },
    { "GET_STATUS",       command_get_status,       "No argument", "PLAYING, PAUSED or CONTENTLESS, with the system and the content's file name.", CMD_INFO_READ_ONLY, NULL, &cmd_rows_status },
-#ifdef HAVE_SCREENSHOTS
+#if defined(HAVE_SCREENSHOTS) && defined(HAVE_RPNG)
    { "GET_FRAME",        command_get_frame,        "[size]", "The content's frame on screen now, as an image scaled down to fit size; nothing is written. For what memory cannot say - a game whose RAM is not charted, or checking it.", CMD_INFO_READ_ONLY, cmd_params_get_frame },
 #endif
    { "GET_OPTION",       command_get_option,       "[option]", "Report an option's value, or list every option with its value.", CMD_INFO_READ_ONLY | CMD_INFO_NO_TOOL, cmd_params_option, &cmd_rows_option },

docs/mcp-server.md

@@ -610,7 +610,9 @@ the layout RetroArch's achievements use when they know no console.
 `GET_MEMORY_MAP` lists the two as `system` and `save`, in the address
 space `core`. These are offsets into the core's RAM, not the console's
 addresses: the SNES's work RAM at `$7E0000` is `0` here, which is also the
-address RetroAchievements' notes give for it. Reads run across from one
+address RetroAchievements' notes give for it. Other consoles' notes may
+place RAM elsewhere - the NES's save RAM at `0x6000`, say, is straight
+after system RAM here. Reads run across from one
 to the other, `WATCH` and conditions take them, and `WRITE_CORE_MEMORY`
 writes them.
 

input/input_driver.c

@@ -16553,9 +16553,7 @@ void input_driver_poll(void)
    }
 
 #ifdef HAVE_COMMAND
-#ifdef HAVE_MENU
-   command_menu_closed();
-#endif
+   command_frame_start();
    /* The interfaces outlive a content load, but the core they would
     * reach does not: what arrives meanwhile waits for it. */
    for (i = 0; i < ARRAY_SIZE(input_st->command)

samples/network/command_frames/command_frames_test.c

@@ -1293,7 +1293,7 @@ int main(int argc, char **argv)
       CHECK(command_steps(&cmd,"|on") && !strcmp(reply,"QUEUED 0\n")
             && !(loop->frame_work & RUNLOOP_WORK_RUN_FRAMES));
       /* so does achievements' hardcore mode taking effect, which lets
-       * go of held input too */
+       * go of held input too, and says why the queue ended */
       reply[0]=0;
       CHECK(command_steps(&cmd,"run 100|on"));
       runloop_iterate();
@@ -1303,7 +1303,8 @@ int main(int argc, char **argv)
       CHECK(input_driver_command_held(0,axes) == 0
             && !(loop->frame_work & RUNLOOP_WORK_RUN_FRAMES));
       reply[0]=0;
-      CHECK(command_steps(&cmd,"|on") && !strcmp(reply,"QUEUED 0\n"));
+      CHECK(command_steps(&cmd,"|on") && !strcmp(reply,
+            "QUEUED 0\nPREVIOUS ERROR achievements hardcore mode is on\n"));
       /* idle is time the content sat paused: a frame it runs ends it */
       reply[0]=0;
       CHECK(command_steps(&cmd,"run 1|on"));

tasks/task_screenshot.c

@@ -816,7 +816,8 @@ static bool screenshot_dump(
          return false;
       }
 
-      task->type         = TASK_TYPE_BLOCKING;
+      /* a frame writes no file, so it waits on no screenshot */
+      task->type         = frame_max ? TASK_TYPE_NONE : TASK_TYPE_BLOCKING;
       task->state        = state;
       task->handler      = task_screenshot_handler;
       if (savestate || frame_max)
@@ -1153,7 +1154,9 @@ static bool take_screenshot_or_frame(
             || video_driver_cached_frame_is_hw_render())
          prefer_vp_read           = true;
       /* Avoid GPU screenshots with savestates */
-      if (video_gpu_screenshot && !savestate)
+      /* nor with a frame: a software core's own frame needs no read
+       * back from the GPU, nor the present a read back asks for */
+      if (video_gpu_screenshot && !savestate && !frame_max)
          prefer_vp_read           = true;
    }
    /* No way to infer screenshot directory. */