Only allow pausing when there is an active core

[alanide]

Guidelines

1. Rebase before opening a pull request
2. If you are sending several unrelated fixes or features, use a branch and a separate pull request for each
3. If possible try squashing everything in a single commit. This is particularly beneficial in the case of feature merges since it allows easy bisecting when a problem arises

Description

Please see this forum link for details.

TLDR: This change blocks the pause action from taking effect when there is no active core. And forces an unpause action upon the closing of an active core.

The intended result of which is a more consistent transition between game launches. Specifically when a custom input_overlay configuration has been defined which keys off of the pause action. For example:

overlays = 2
overlay0_overlay = "~/Library/Application Support/RetroArch/overlays/pause/empty.png"
overlay0_name = "pause"
overlay0_descs = 0

overlay1_overlay = "NES.png"
overlay1_rect = "0.0,0.0,1.0,1.0"
overlay1_full_screen = true
overlay1_descs = 1
overlay1_desc0 = "overlay_next,1,1,rect,1,1"
overlay1_desc0_next_target = "pause"

Prior to this change, toggling pause without a core being loaded could result in the pause action being 180° out of phase with the pause overlay being displayed.

PS - I apologize in advance if I am stepping on anyone's toes, or if this is the incorrect way to introduce such a change. I am a software engineer by trade, but it has probably been a decade or more since my last contribution to an open source project. And probably 2 decades since I last wrote anything in C... 🙃

Related Issues

There is one additional manifestation of this issue. Specifically when 'hot swapping' between games via the quick menu, without first closing the open content. I was unable to determine how to properly resolve that without introducing unintended side effects. Suggestions are welcome.

Related Pull Requests

[Any other PRs from related repositories that might be needed for this pull request to work]

Reviewers

[If possible @mention all the people that should review your pull request]

[LibretroAdmin]

Hi @neil4 , what do you think of this?

[hizzlekizzle]

I think @sonninnos might have some thoughts, as well. IIRC, he was talking to someone recently who complained about pause behavior being unpredictable/unintuitive coming in and out of menus.

[neil4]

I'm not sure I understand the overlay example, but the description sounds reasonable as far as general pause toggle behavior.

I'm miles from my PC, but I can look at this in a couple of days.

[neil4]

@alanide Ok, one suggestion, maybe two.

In the CMD_EVENT_PAUSE_TOGGLE and CMD_EVENT_PAUSE cases, for clarity I'd leave the old code as is and just break early if the core isn't running, e.g.

case CMD_EVENT_PAUSE_TOGGLE:
     {
        /* Allow pause toggling only when there is an active core. */
        if (!(runloop_st->flags & RUNLOOP_FLAG_CORE_RUNNING))
           break;
        ...

I'm pretty neutral on changing the stay-paused behavior when loading content while paused (hot swapping). Fastforward and slowmotion are the same way, even if most of RetroArch (like the overlay) is reinitialized with new content.

But, you could make it unpaused on start by moving runloop_paused_hotkey somewhere outside of runloop_check_state() (maybe in the runloop struct) and set it false in CMD_EVENT_CORE_DEINIT. Right now it's a static local that survives core deinits, and when new content starts, runloop_check_state() checks it and pauses to enforce the last pause-hotkey press.

[alanide]

Thank you for the feedback. I will make some changes later this week.

[alanide]

@neil4 - I have pushed the requested changes. Thank you for pointing me in the right direction. The runloop_paused_hotkey variable was the puzzle piece that I was missing. The pause behavior feels much more consistent to me now. I hope others feel the same.

I do want to call out that the issue can still manifest if the pause state is changed while the quick-menu is open, and the user returns to the game without restarting, closing, or 'hot swapping'. (ie: no core deinit takes place). But based on my limited understanding of how the quick-menu is intended to work, I don't think changing that behavior would be correct for all use cases. So I can live with it, if everyone else feels the same.

[neil4]

I gave you bad example code -- pause needs to be declared before checking RUNLOOP_FLAG_CORE_RUNNING.

[alanide]

Updated and squashed.

I apologize, I have forgotten the idiosyncrasies of the C language...

[neil4]

One last nit: runloop_pause_toggle() doesn't need runloop_st->paused_hotkey to be passed in.

[alanide]

@neil4 - Updated

[neil4]

LGTM