| Re: [hatari-devel] [PATCH] Add joystick button index configuration |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
- To: hatari-devel@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [hatari-devel] [PATCH] Add joystick button index configuration
- From: Robin Sergeant <robin.sergeant@xxxxxxxxx>
- Date: Sun, 30 Jul 2023 09:33:01 +0100
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1690705993; x=1691310793; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=8i/zzusrrLNsWiZlac02cJKbeP0fxF/+tzvsmpSCa9w=; b=hEAMfqB/jsmvYlmEnO+jmIGoG6L9nfU5zePuHCgBfg5QKBGceHNd0ToqtQtnasPGvz jsQVb+sOzxqGE+WG5+Sj/oRTMXojBVfspCJ71Qbi3Tc6Ku78Y34LpV34DKDuzwd1BQlh YblvdZJ2S+lsZkyU/8hbbzWW9dX78QZVTGYu2MnfgMUdCCX12erd1WQdqxFeCmIiN+5y OxYMZdLtiYgrwCq2ubTd+sAgSdwXdbsCD5davpilpFsNjUc7YS1C8cWSfwgmjzPQcPF7 oe9W/D5dsp5XzDMmGsCiD+Ef/4DPi0SQNqN0qduLXSrqnraxmRdpNFNEGqgpw1TmFszc /0RQ==
Hi,
I've experimented with SDL_GAMECONTROLLERCONFIG, but it only seems to
create game controllers and cannot be used to configure joysticks.
SDL has a separate SDL_GameController interface which is very similar
to SDL_Joystick. I was able to set up a game controller that mapped
to my joystick using it's guid, but the buttons still refused to
change for some reason!
Anyway, going back to my config proposal I've now re-worked that to
include the corresponding sdl gui changes. I will send the two
patches in a new email to avoid confusion.
I had to change joy.c again to cater for undefined buttons - I wanted
to let the user choose no button if for example they have a joystick
with less than 3 buttons. This will be stored using index of -1 which
is fine, but we cannot call SDL_JoystickGetButton() with -1 as they
have no check for negative indexes. Looking at their code -1 would
index past the beginning of an array :-) So I've just made sure we
only call it with non negative indexes.
Br,
Robin.
On Thu, 27 Jul 2023 at 11:13, Eero Tamminen <oak@xxxxxxxxxxxxxx> wrote:
>
> Hi Robin,
>
> On 25.7.2023 2.30, Robin Sergeant wrote:
> > Sorry for the spam, but the patch in my previous email was corrupted by
> > gmail. So re-sending using git send-email to correct that.
>
> Thanks for the patch!
>
>
> > Currently the main joystick button (fire) is hardcoded as SDL button
> > index 0, which is not the best choice for every joystick. For example
> > on my Speedlink Competition Pro the buttons are arranged like this:
> >
> > (3)(0) - main buttons
> > (4)(1) - small buttons behind
> >
> > Button 0 is therefore only the correct choice for left handed players,
> > and others (like me!) would prefer to use button 3. As this was
> > driving me nuts I've created a patch that you might like to include?
>
> While your patch looks fine, configuring joysticks is not Hatari
> specific problem, it applies to all programs using joysticks.
>
> Therefore such configuring is expected to be done outside / before
> starting Hatari, using some OS specific utility for it.
>
> Many years ago when Hatari still used SDL1, I've used this for
> calibration on Linux:
> https://jstest-gtk.gitlab.io/
>
> But nowadays things seem to work differently. Here are some notes on
> how to do it now, and with SDL2:
> https://wiki.archlinux.org/title/Gamepad#Remapping_of_gamepad_buttons_and_more
>
>
> > It adds 3 new integers config values for each Joystick, e.g.
> >
> > [Joystick1]
> > ...
> > nJoyBut1Index = 3
> > nJoyBut2Index = 4
> > nJoyBut3Index = 0
> >
> > NB But2 is the space/jump button, and But3 is the auto fire button.
> > Defaults are 0, 1 and 2 (matching the current behaviour). I haven't
> > added these to the UI, but the config file can be manually edited to
> > swap the buttons.
>
> As configuring joysticks outside of Hatari seems more complicated
> nowadays, doing it in Hatari itself could be considered, but then it
> would definitely need to have support in SDL GUI, so that Hatari users
> know about it & can use it easily.
>
> What the others think? Thomas? Nicolas?
>
>
> Because SDL GUI changes are quite a bit of work, it's best to first mail
> an ASCII proposal of what the new GUI layout would look like.
>
> Or, you could just test how SDL2 joystick mapping works, and contribute
> documentation update for that...
>
>
> > Any comments or questions?
>
> Some generic comments on contributions:
>
> Changes to different functionality (e.g. config support for joy button
> mappings, and GUI support for them) should be in separate patches, as
> should any white-space changes.
>
> Patches should be sent as attachment (not inline, like they're now) so
> they can be saved & applied directly.
>
> It's better to commit the changes to a local tree first and use
> something like "git format-patch HEAD~1" [1] to produce the patch(es),
> so they can be applied directly to Hatari Git with correct attributions.
>
> Proposals for following documentation updates would also be great:
> * Note about the new feature for release-notes.txt
> * Contributors list update for authors.txt
>
> (We may slightly update the doc texts & commit descriptions before
> pushing the changes.)
>
>
> - Eero
>
> [1] Number after "HEAD~" indicates how many commits from the git branch
> top you want to include to given patch set.
>
> > Br,
> >
> > Robin
> > ---
> > src/configuration.c | 25 +++++++++++++++++++++++--
> > src/includes/configuration.h | 1 +
> > src/joy.c | 14 +++++++++-----
> > 3 files changed, 33 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/configuration.c b/src/configuration.c
> > index de6006d0..e07b7327 100644
> > --- a/src/configuration.c
> > +++ b/src/configuration.c
> > @@ -108,6 +108,9 @@ static const struct Config_Tag configs_Joystick0[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[0].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[0].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[0].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[0].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[0].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[0].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[0].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[0].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[0].nKeyCodeLeft },
> > @@ -123,6 +126,9 @@ static const struct Config_Tag configs_Joystick1[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[1].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[1].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[1].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[1].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[1].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[1].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[1].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[1].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[1].nKeyCodeLeft },
> > @@ -138,6 +144,9 @@ static const struct Config_Tag configs_Joystick2[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[2].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[2].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[2].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[2].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[2].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[2].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[2].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[2].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[2].nKeyCodeLeft },
> > @@ -153,6 +162,9 @@ static const struct Config_Tag configs_Joystick3[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[3].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[3].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[3].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[3].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[3].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[3].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[3].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[3].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[3].nKeyCodeLeft },
> > @@ -168,6 +180,9 @@ static const struct Config_Tag configs_Joystick4[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[4].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[4].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[4].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[4].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[4].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[4].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[4].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[4].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[4].nKeyCodeLeft },
> > @@ -183,6 +198,9 @@ static const struct Config_Tag configs_Joystick5[] =
> > { "bEnableAutoFire", Bool_Tag, &ConfigureParams.Joysticks.Joy[5].bEnableAutoFire },
> > { "bEnableJumpOnFire2", Bool_Tag, &ConfigureParams.Joysticks.Joy[5].bEnableJumpOnFire2 },
> > { "nJoyId", Int_Tag, &ConfigureParams.Joysticks.Joy[5].nJoyId },
> > + { "nJoyBut1Index", Int_Tag, &ConfigureParams.Joysticks.Joy[5].nJoyBut1Index },
> > + { "nJoyBut2Index", Int_Tag, &ConfigureParams.Joysticks.Joy[5].nJoyBut2Index },
> > + { "nJoyBut3Index", Int_Tag, &ConfigureParams.Joysticks.Joy[5].nJoyBut3Index },
> > { "kUp", Key_Tag, &ConfigureParams.Joysticks.Joy[5].nKeyCodeUp },
> > { "kDown", Key_Tag, &ConfigureParams.Joysticks.Joy[5].nKeyCodeDown },
> > { "kLeft", Key_Tag, &ConfigureParams.Joysticks.Joy[5].nKeyCodeLeft },
> > @@ -609,6 +627,9 @@ void Configuration_SetDefault(void)
> > ConfigureParams.Joysticks.Joy[i].bEnableAutoFire = false;
> > ConfigureParams.Joysticks.Joy[i].bEnableJumpOnFire2 = true;
> > ConfigureParams.Joysticks.Joy[i].nJoyId = (i > maxjoy ? maxjoy : i);
> > + ConfigureParams.Joysticks.Joy[i].nJoyBut1Index = 0;
> > + ConfigureParams.Joysticks.Joy[i].nJoyBut2Index = 1;
> > + ConfigureParams.Joysticks.Joy[i].nJoyBut3Index = 2;
> > ConfigureParams.Joysticks.Joy[i].nKeyCodeUp = SDLK_UP;
> > ConfigureParams.Joysticks.Joy[i].nKeyCodeDown = SDLK_DOWN;
> > ConfigureParams.Joysticks.Joy[i].nKeyCodeLeft = SDLK_LEFT;
> > @@ -631,12 +652,12 @@ void Configuration_SetDefault(void)
> > ConfigureParams.Keyboard.nKbdLayout = TOS_LANG_UNKNOWN;
> > ConfigureParams.Keyboard.nLanguage = TOS_LANG_UNKNOWN;
> > strcpy(ConfigureParams.Keyboard.szMappingFileName, "");
> > -
> > +
> > /* Set defaults for Shortcuts */
> > ConfigureParams.Shortcut.withoutModifier[SHORTCUT_OPTIONS] = SDLK_F12;
> > ConfigureParams.Shortcut.withoutModifier[SHORTCUT_FULLSCREEN] = SDLK_F11;
> > ConfigureParams.Shortcut.withoutModifier[SHORTCUT_PAUSE] = SDLK_PAUSE;
> > -
> > +
> > ConfigureParams.Shortcut.withModifier[SHORTCUT_DEBUG] = SDLK_PAUSE;
> > ConfigureParams.Shortcut.withModifier[SHORTCUT_OPTIONS] = SDLK_o;
> > ConfigureParams.Shortcut.withModifier[SHORTCUT_FULLSCREEN] = SDLK_f;
> > diff --git a/src/includes/configuration.h b/src/includes/configuration.h
> > index 86e26c37..fa92760a 100644
> > --- a/src/includes/configuration.h
> > +++ b/src/includes/configuration.h
> > @@ -169,6 +169,7 @@ typedef struct
> > bool bEnableAutoFire;
> > bool bEnableJumpOnFire2;
> > int nJoyId;
> > + int nJoyBut1Index, nJoyBut2Index, nJoyBut3Index;
> > int nKeyCodeUp, nKeyCodeDown, nKeyCodeLeft, nKeyCodeRight, nKeyCodeFire;
> > } JOYSTICK;
> >
> > diff --git a/src/joy.c b/src/joy.c
> > index 3fd0b884..10d1fae0 100644
> > --- a/src/joy.c
> > +++ b/src/joy.c
> > @@ -194,8 +194,12 @@ void Joy_UnInit(void)
> > * Read details from joystick using SDL calls
> > * NOTE ID is that of SDL
> > */
> > -static bool Joy_ReadJoystick(int nSdlJoyID, JOYREADING *pJoyReading)
> > +static bool Joy_ReadJoystick(int nStJoyId, JOYREADING *pJoyReading)
> > {
> > + int nSdlJoyID = ConfigureParams.Joysticks.Joy[nStJoyId].nJoyId;
> > + int button1 = ConfigureParams.Joysticks.Joy[nStJoyId].nJoyBut1Index;
> > + int button2 = ConfigureParams.Joysticks.Joy[nStJoyId].nJoyBut2Index;
> > + int button3 = ConfigureParams.Joysticks.Joy[nStJoyId].nJoyBut3Index;
> > unsigned hat = SDL_JoystickGetHat(sdlJoystick[nSdlJoyID], 0);
> >
> > /* Joystick is OK, read position from the configured joystick axis */
> > @@ -211,12 +215,12 @@ static bool Joy_ReadJoystick(int nSdlJoyID, JOYREADING *pJoyReading)
> > if (hat & SDL_HAT_DOWN)
> > pJoyReading->YPos = 32767;
> > /* Sets bit #0 if button #1 is pressed: */
> > - pJoyReading->Buttons = SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], 0);
> > + pJoyReading->Buttons = SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], button1);
> > /* Sets bit #1 if button #2 is pressed: */
> > - if (SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], 1))
> > + if (SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], button2))
> > pJoyReading->Buttons |= JOYREADING_BUTTON2;
> > /* Sets bit #2 if button #3 is pressed: */
> > - if (SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], 2))
> > + if (SDL_JoystickGetButton(sdlJoystick[nSdlJoyID], button3))
> > pJoyReading->Buttons |= JOYREADING_BUTTON3;
> >
> > return true;
> > @@ -308,7 +312,7 @@ uint8_t Joy_GetStickData(int nStJoyId)
> > }
> >
> > /* Read real joystick and map to emulated ST joystick for emulation */
> > - if (!Joy_ReadJoystick(nSdlJoyId, &JoyReading))
> > + if (!Joy_ReadJoystick(nStJoyId, &JoyReading))
> > {
> > /* Something is wrong, we cannot read the joystick from SDL */
> > bJoystickWorking[nSdlJoyId] = false;
>
>