Skip to content
This repository has been archived by the owner on Aug 8, 2022. It is now read-only.

3ds-SDL: multiple fixes #57

Merged
merged 3 commits into from May 31, 2020
Merged

Conversation

sergiou87
Copy link
Collaborator

This PR fixes a bunch of bugs that I found while porting https://github.com/sergiou87/open-supaplex to 3DS:

  • The Start button didn't work at all. The number of buttons of SDL_Joystick was set to 8... but the indices used for the buttons were 1-8, and Start had the 8. Changed Start to use 0 instead.
  • It also adds support for the D-pad buttons and the ZL/ZR buttons (New 3DS only). Before this, the only way to detect the D-pad buttons was through the keyboard API…
  • Another change related to the previous one: stop mapping 3DS buttons into keyboard keys. Seems like that was an ad-hoc solution for the specific game (OpenTyrian) that was ported using this SDL port, instead of mapping the buttons on the game code. But this is just a theory 😛
  • It also fixes the SetColors function for 3DS, which assumed the input colors array was a 256 colors array, but it's not. That parameter has ncolors inside.

@@ -74,13 +74,31 @@ void SDL_SYS_JoystickUpdate (SDL_Joystick *joystick) {
SDL_PrivateJoystickButton (joystick, 7, SDL_PRESSED);
}
if ((key_press & KEY_START)) {
SDL_PrivateJoystickButton (joystick, 8, SDL_PRESSED);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is why the Start button didn't work… I'm surprised nobody noticed it until now 😂

pos= 1<<i;
if(hidkey&pos) keymap[i]=key;
}
// Do nothing
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This function can't be removed, SDL relies on it.

SDL_PrivateKeyboard (SDL_RELEASED, &keysym);
}
}

if (hidKeysHeld() & KEY_TOUCH) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not super happy about it, but I guess it doesn't hurt, so I left the touchscreen mapping to the mouse API. Proper support for touchscreens was added to SDL2, so I guess this is better than nothing.

keymap[0]=SDLK_a; //KEY_A
keymap[1]=SDLK_b; // KEY_B
keymap[2]=SDLK_ESCAPE; //KEY_SELECT
keymap[3]=SDLK_RETURN; //KEY_START
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Random button-to-key mapping like this makes no sense in a SDL port IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Good call. This belongs in the port using SDL, not SDL itself.

Choose a reason for hiding this comment

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

Yep, I also had to work around this in OpenJazz.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for reporting that so we could do something about it @carstene1ns. Much appreciated.

Choose a reason for hiding this comment

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

Well, I found no sane way to have this backward compatible. This breaks quite a lot homebrew this way unfortunately.

Right now you will get an undefined reference, because include/SDL_keyboard.h was not updated to remove the function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But I didn't remove the implementation either, because otherwise it crashes in runtime. I left the implementation empty: https://github.com/devkitPro/SDL/pull/57/files#r432952006

Is that what you were referring to?

Choose a reason for hiding this comment

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

SDL_N3DSKeyBind() is gone.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😱 You're right, that bit didn't make it into this PR, not sure how it went missing. I'm AFK right now but I will fix it ASAP

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh I didn't miss it when I cherry picked the changes, I never deleted it. However, it doesn't fail with undefined reference because it's never used. Anyway, I'll clean it up in a PR in a few hours, sorry about that!

@WinterMute WinterMute merged commit 567aa95 into devkitPro:3ds-sdl-1.2 May 31, 2020
@WinterMute
Copy link
Member

Nice job. Was worth keeping the individual commits as well so I rebased rather than the usual squash.

palette[i] = N3DS_MAP_RGB(colors[i].r, colors[i].g, colors[i].b);
{
int colorIndex = i - firstcolor;
palette[i] = N3DS_MAP_RGB(colors[colorIndex].r, colors[colorIndex].g, colors[colorIndex].b);

Choose a reason for hiding this comment

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

Good find!

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

Successfully merging this pull request may close these issues.

None yet

3 participants