Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a button to quickly swap between Memory<->Rumble Paks #509

Closed
wants to merge 1 commit into from

Conversation

bslenul
Copy link
Contributor

@bslenul bslenul commented Dec 14, 2023

This PR adds a button to quickly swap from Memory Pak to Rumble Pak and vice-versa (RetroPad Select by default, or R3 if "Independent C-button Controls" is enabled):

mupen_swap_btn.mp4

Very useful for games compatible with both Paks, going back and forth in the Quick Menu every time you want to change quickly becomes annoying...

Just select "memory/rumble (swappable)" for any of the "Player N Pak" core options and it will be enabled for that controller. Also added a core option to select if the swap should be done on press or on hold (1, 2 or 3 seconds).

I hope this is fine, if it is then it closes #502

Also removed the update_controllers() call here:

if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE_UPDATE, &updated) && updated) {
update_variables(false);
update_controllers();
}
since it's already done at the end of update_variables() just above.

Comment on lines 1848 to 1857
if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && var.value)
{
if (!strcmp(var.value, "no delay"))
swap_pak_delay = 0;
else if (!strcmp(var.value, "1s hold"))
swap_pak_delay = 60;
else if (!strcmp(var.value, "2s hold"))
swap_pak_delay = 120;
else if (!strcmp(var.value, "3s hold"))
swap_pak_delay = 180;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The heck is that unit? Frames?
  2. If its Frames/VI, then this is not correct depending on Buffer Swap mode for GlideN & PAL/NTSC configurations
  3. atoi or sscanf, prefer not to have val and string mixed tho

Copy link
Contributor Author

@bslenul bslenul Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it was supposed to be frames, I assumed 60fps and completely forgot about PAL 😓
A bit annoying since ROM_PARAMS isn't initialized yet on the first update_variables() pass so it returns 0 (NTSC) as region even with a PAL game...

Would that be OK as a workaround?

    var.key = CORE_NAME "-pak-swap-delay";
    var.value = NULL;
    if (environ_cb(RETRO_ENVIRONMENT_GET_VARIABLE, &var) && var.value)
    {
       // If startup is true then ROM infos aren't ready yet and we don't know
       // the region of the game yet, so we just keep the seconds for now
       // and we'll calculate in frames a bit later in retro_load_game()
       swap_pak_delay = atoi(var.value);
       if (!startup)
          swap_pak_delay *= retro_get_region() == RETRO_REGION_PAL ? 50 : 60;
    }

then in retro_load_game() after we got the ROM infos:

    // Now that we know the region, we can calculate the seconds in frames
    // for the 'Memory/Rumble Pak Swap' button hold delay
    swap_pak_delay *= retro_get_region() == RETRO_REGION_PAL ? 50 : 60;

and I changed the values in libretro_core_options.h to:

            {"0", "0 second"},
            {"1", "1 second"},
            {"2", "2 seconds"},
            {"3", "3 seconds"},

so even if it shouldn't have been an issue with atoi, there's no val/string mix.

As for the buffer swap I'm not sure how to handle that tbh 🤔

A bit of a bummer, looks more complicated than I expected for my very limited C/C++ skills :(

Copy link
Contributor Author

@bslenul bslenul Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes pushed, AFAICT it works fine with the correct hold duration for NTSC/PAL now.

Still not sure to understand your "buffer swap" comment, doesn't retro_run run 60 (or 50 for PAL) times per second no matter what? Because basically the only thing I'm doing is once per retro_run I'm increasing the frame count for how long the button to swap paks has been held.

Copy link
Contributor Author

@bslenul bslenul Dec 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different approach using clock() from time.h so the duration of the button being held is calculated in actual seconds instead of relying on frames: bslenul@e2d0392

Otherwise I could simply drop the hold button feature and make the swap on button press only. I initially added the hold options to avoid misclicks, but if you think it's not worth I don't mind removing it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will think of smth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

controller pack hotkey
2 participants