Add basic gamepad support and button remapping #35

techknight · 2023-12-04T02:26:11Z

Summary

Gamepad support version 1! Players can use their gamepads to fly the plane during gameplay, and remap buttons in the options screen.

In Xbox terms, these default controls have been set up:

X: Fire machinegun
Y: Flip
A: Drop bomb
LB/RB: Decelerate/Accelerate
Menu: Go home

Compatibility

These setups have been tested as working in gameplay and in remapping:

Xbox One gamepad (Bluetooth) on Windows, MacOS, and Linux
Xbox Elite v2 gamepad (Xbox Wireless protocol) on Windows
8bitdo Micro gamepad (Bluetooth, Mode D, d-pad) on Linux

Implementation

gamepad.c and gamepad.h have been added to handle most gamepad input during gameplay.

The most recent input device is tracked, to keep button handling from interfering with keyboard handling.

Gamepad Bindings

The options screen now supports remapping gamepad buttons:

Limitations

In this first version, only digital input is supported, no analog sticks or triggers
A limited number of gamepads have been tested

Future versions

Analog inputs could be supported, but it will require more complex input handling and a new way of saving configuration values
Navigating menus using the gamepad will require discussion, because it would radically change how menus are implemented

fragglet

Thanks for doing this!

fragglet · 2023-12-04T15:44:55Z

src/gamepad.h

+void Gamepad_Update(void);
+void Gamepad_CheckState(void);
+
+extern int btnbindings[NUM_KEYS];


NUM_KEYS is the number of keyboard bindings. You'll want to define a new one, eg. NUM_GAMEPAD_BUTTONS

fragglet · 2023-12-04T15:53:19Z

src/swconf.h

 	} type;
 	union {
 		void *v;
 		bool *b;
 		int *i;
+		int *btn;


This seems to be unused. I think you're doing the right thing by just using .i, but you can get rid of this field.

I ended up keeping this for the moment: there's two spots in drawmenu() and runmenu() where we need to know whether we're in the keyboard menu, or in the gamepad menu, in order to do the correct action. (ie checking for CONF_KEY vs CONF_BTN)

I did play around with another way where CONF_BTN was completely removed, where it was just like: if (!strcmp(title, "OPTIONS > GAMEPAD BINDINGS")) { 😅, but maybe I could create a new MENU_TYPE that gets passed to runmenu when it gets executed? That might be the cleanest, and get rid of using CONF_BTN altogether

fragglet · 2023-12-04T15:54:14Z

src/sdl/swconf.c

+			sprintf(btnString, "%d", *opt->value.i); // Convert integer to string
+			swputs(btnString);
+			break;
+		}


Please remove the {} braces here; they are misleading.

fragglet · 2023-12-04T15:58:29Z

src/Makefile.am

        pcsound.h                  \
        std.h                      \
        swasynio.c   swasynio.h    \
        swauto.c     swauto.h      \
        swcollsn.c   swcollsn.h    \
-        swconf.c     swconf.h      \


Please move swconf.c and swinit.c back to their previous location, they don't belong in the sdl/ subdirectory.

I understand why you had to do this: the new feature requires using new SDL functions. But the correct way to do this is to create accessor functions to access them. For example, we have Vid_GetKey() that is used by the configuration code to read a keypress. You need a similar function that your change_btn_binding function can call to read a gamepad press.

In case it's not clear: the code in src/ is the "core code" of the game. The idea is that if someone wants to port the game to a new system where SDL isn't available, they should just be able to reimplement the files in src/sdl/ . Think of it as though there were multiple other directories alongside src/sdl/ - we wouldn't want to contain duplicate implementations of the initialization and configuration code for every one of them.

My bad - I should've asked in Discord for some more context. This is all moved back and straightened out.

fragglet · 2023-12-04T16:02:02Z

src/sdl/gamepad.c

+
+void Gamepad_Update(void) {
+    if (!gamepad_initted || gamepad == NULL) {
+        Gamepad_Init();


We don't need to try to initialize again every time the function is called. Let's just return here.

fragglet · 2023-12-04T16:06:40Z

src/sdl/gamepad.c

+	if (!gamepad_initted) {
+		return;
+	}
+    // Close the gamepad


Can you try to be more consistent with your formatting? Not just talking about this one function but throughout the PR. You can see here that it's kind of a mess because you mixed up spaces and tabs.

The coding style for the project is to use tabs for indentation. If you configure your editor to not expand tabs to spaces then you'll probably get the right thing.

Oof, sorry about that, I've fixed my settings there

…andling

techknight · 2023-12-06T03:12:14Z

In the changes today, I've also added support for looking up the button names so that they look nice now in the button config menu ✨

fragglet · 2023-12-20T00:35:11Z

src/sdl/gamepad.c

+	while (btn == -1) {
+		if (SDL_WaitEventTimeout(&event, 10)) {
+			if (event.type == SDL_CONTROLLERBUTTONDOWN) {
+				btn = event.cbutton.button;


Justreturn event.cbutton.button; and you can eliminate the btn variable.

fragglet · 2023-12-20T00:54:56Z

src/sdl/gamepad.c

+{
+	SDL_Event event;
+
+	while (SDL_PollEvent(&event)) {


Doing this means that there are now two event loops: the one in the gamepad code and the one in the video code. This will mean that some events get discarded mistakenly. You need to use a single combined event loop.

fragglet · 2023-12-20T00:55:34Z

src/sdl/gamepad.c

+{
+	if (SDL_Init(SDL_INIT_GAMECONTROLLER) < 0) {
+		printf("Gamepad could not initialize! SDL_Error: %s\n", SDL_GetError());
+	} else {


else if (...

fragglet · 2023-12-20T00:57:25Z

src/sdl/gamepad.c

+				btn = event.cbutton.button;
+				break;
+			} else if (event.type == SDL_KEYDOWN) {
+				if (event.key.keysym.sym == SDLK_ESCAPE) {


You can combine the two if statements with an &&

fragglet · 2023-12-20T00:59:44Z

src/sdl/gamepad.c

+	last_input_gamepad = input;
+}
+
+int isGamepadInitted(void) {


You can get rid of this function; just add a check to Gamepad_GetGameBtns that returns 0 if !gamepad_initted.

fragglet · 2023-12-20T01:01:08Z

src/swmain.c

 	int tictime;

 	/* generate a new move command and save it */

 	multkey = Vid_GetGameKeys();
+	if(isGamepadInitted()) {


Is there any reason you can't just:

multkey = Vid_GetGameKeys() | Gamepad_GetGameBtns();

?

Then I think you can get rid of all the "LastInputGamepad" stuff entirely.

fragglet · 2023-12-20T01:03:00Z

src/sdl/video.c

@@ -536,12 +533,12 @@ void Vid_SetVideoPalette(int palette)

 const char* Vid_GetVideoPaletteName(int palette)
 {
-        return VideoPalettes[palette].name;
+		return VideoPalettes[palette].name;


This is too much indentation. Side note, can you try to avoid including unrelated changes?

techknight added 10 commits December 1, 2023 16:54

Test if gamepad detected

24fd6f8

Test if gamepad detected 2

f324e8e

Test if gamepad detected 3

d2d579e

Test if gamepad connected 4

383bef2

Merge branch 'fragglet:trunk' into gamepad-test

d820241

Move button handling to gamepad.c

3716a5c

Simplify gamepad handling and detect input type last used

afa49d4

Add basic gamepad support and button remapping

8fcf1e8

Fix handling of Gamepad_Update()

dbbebdb

Fix handling of SDL.h

c4b11c3

fragglet requested changes Dec 4, 2023

View reviewed changes

Add requested changes and better separation of gamepad and keyboard h…

1f3f9f2

…andling

techknight requested a review from fragglet December 6, 2023 03:08

fragglet requested changes Dec 20, 2023

View reviewed changes

fragglet force-pushed the trunk branch from 0473073 to 33eab06 Compare November 11, 2024 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic gamepad support and button remapping #35

Add basic gamepad support and button remapping #35

techknight commented Dec 4, 2023 •

edited

Loading

fragglet left a comment

fragglet Dec 4, 2023

techknight Dec 6, 2023

fragglet Dec 4, 2023

techknight Dec 6, 2023 •

edited

Loading

fragglet Dec 4, 2023

techknight Dec 6, 2023

fragglet Dec 4, 2023

techknight Dec 6, 2023

fragglet Dec 4, 2023

techknight Dec 6, 2023

fragglet Dec 4, 2023

techknight Dec 6, 2023

techknight commented Dec 6, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

fragglet Dec 20, 2023

Add basic gamepad support and button remapping #35

Are you sure you want to change the base?

Add basic gamepad support and button remapping #35

Conversation

techknight commented Dec 4, 2023 • edited Loading

Summary

Compatibility

Implementation

Gamepad Bindings

Limitations

Future versions

fragglet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techknight Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techknight commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techknight commented Dec 4, 2023 •

edited

Loading

techknight Dec 6, 2023 •

edited

Loading