ExOK / Celeste64

A game made by the Celeste developers in a week(ish, closer to 2)
1.59k stars 125 forks source link

Game doesn't handle Switch compatible controllers on Linux, only XBOX #20

Closed afontenot closed 5 months ago

afontenot commented 5 months ago

For clarity, when I say "Switch compatible", I mean my 8BitDo SN30 pro+ in Switch mode. I haven't tested any other Switch controllers. These controllers have a button layout like this:

  X
Y   A
  B

Usually when games (using SDL I guess, or some library that wraps SDL?) detect a controller with this configuration, they set B to confirm / jump / etc, and A to cancel. Celeste does this, for example.

Celeste 64 appears to recognize that my "B" button is a "B" button, but treats it as cancel. So if I press this button on the start screen, the game quits instead of starting.

My controller has the option to switch to XBOX mode with a key combination, and when I do that it works correctly with Celeste 64.

This might be an issue on Windows too, but I haven't tested it.

coolreader18 commented 5 months ago

For others finding this annoying, as a temporary fix I played through the entirety of Celeste 64 with a switch pro controller with this 2 line patch:

diff --git a/Source/Controls.cs b/Source/Controls.cs
index b6e3981..44aa290 100644
--- a/Source/Controls.cs
+++ b/Source/Controls.cs
@@ -26,11 +26,11 @@ public static class Controls
        Camera.Add(Keys.A, Keys.D, Keys.W, Keys.S);

        Jump.Clear();
-       Jump.Add(0, Buttons.A, Buttons.Y);
+       Jump.Add(0, Buttons.B, Buttons.X);
        Jump.Add(Keys.C);

        Dash.Clear();
-       Dash.Add(0, Buttons.X, Buttons.B);
+       Dash.Add(0, Buttons.Y, Buttons.A);
        Dash.Add(Keys.X);

        Climb.Clear();

For actually fixing it, it might be a bit tricky, since it seems like Foster only exposes "A" "B" "X" and "Y", and not e.g. BTN_SOUTH like linux evdev does. That would be ideal, to say "confirm uses A but jump uses the button on the bottom and the button on the top". But that might be tricky, since SDL only has ABXY too.

NoelFB commented 5 months ago

Yeah, so the way Foster and SDL2 work is that while the buttons are called A,B,X,Y, they are functionally South,East,West,North. The part that I think is trickier is that it needs to change the bindings depending on which controller you have plugged in. At the moment it should display the correct buttons, but Switch will just be flipepd (ex. B confirms and A cancels). However, I'm not certain if the current controller detection is actually working (ex. if it still displays xbox images even if you're using a switch controller) which would be a separate issue.

So problem 1) is that I think the "default" bindings need to be different depending on the controller detected, and problem 2) is that it may not be detecting the controller at all and falling back to showing xbox images.

(also in general I'm in favor of updating Foster to use North/South/East/West, as SDL3 is looking to do that too)

coolreader18 commented 5 months ago

However, I'm not certain if the current controller detection is actually working (ex. if it still displays xbox images even if you're using a switch controller) which would be a separate issue.

It does indeed display xbox buttons, and I did some digging to figure out why - it seems like for some reason Foster or I guess SDL doesn't properly get the Vendor and Product ids of the device (they're just 0), even though other parts of my system and even sdl2-jstest (joystick test) do indeed show a pro controller as 0x057e, 0x2009.

Switch will just be flipped (ex. B confirms and A cancels)

You might just be talking about the images, but for me on a pro controller without that patch, A confirms and B cancels, but the movement bindings were wrong (B dashes and A/Y jumps).

NoelFB commented 5 months ago

Vendor and Product ids of the device (they're just 0)

To me this sounds like a bug with Foster then - SDL is likely correct. Is there a chance you have multiple USB devices that are considered controllers? We only poll the first one right now. I'll take a look though with a Switch controller and see if it gets the proper values!

You might just be talking about the images, but for me on a pro controller without that patch, A confirms and B cancels, but the movement bindings were wrong (B dashes and A/Y jumps).

Hmm that's interesting. From my understanding a Switch controller right now, B should be confirm and A should be cancel.... unless it's somehow being remapped in SDL2 for me and I just didn't realize.

NoelFB commented 5 months ago

Yeah I'm getting 0's for everything too. Interestingly it does initially load with proper values, but then gets unset. Looking into a fix for at least that in Foster.

NoelFB commented 5 months ago

Alright I fixed at least part of this issue here https://github.com/FosterFramework/Foster/commit/b93c0bdd775a02037f1a2f3d6809493a5e22817f. Can be tested if you clone Foster locally and tell Celeste to use your local version (via a ProjectReference instead of NuGet PackageReference).

Looking into possible bindings being weird next. Cancel should be A and Confirm should be B on Switch. Once that's fixed then ideally the bindings would be different depending on the controller connected.

coolreader18 commented 5 months ago

Yep, it works now! Though Nintendo Switch/confirm.png is the B button, even though SDL maps pro controller B to SDL_CONTROLLER_BUTTON_B, so pressing B does actually exit the game.

afontenot commented 5 months ago

My controller doesn't report a Nintendo vendor, so Foster still doesn't recognize it and defaults to XBOX layout. I have vendor 0x2dc8 and product 0x6002.

NoelFB commented 5 months ago

@afontenot what is the actual controller that you're using?

afontenot commented 5 months ago

@afontenot what is the actual controller that you're using?

8BitDo SN30 pro+, in its "native" / Switch compatible mode. It's a retro style controller with Nintendo button layout. (It also has two modes where it emulates either a PS or XBOX controller, but these are not in use.) Vanilla Celeste prints this to the terminal: Controller 0: 8BitDo SN30 Pro+.

https://support.8bitdo.com/faq/sn30-pro-plus.html

NoelFB commented 5 months ago

Hmm OK, right now this is how the controller is decided. It feels like we could continue to add more here, but it might also be nice to have a way to hook more up dynamically: https://github.com/FosterFramework/Foster/blob/main/Framework/Input/Controller.cs#L121

coolreader18 commented 5 months ago

Would something like this make sense for controller-dependent bindings?

diff --git a/Source/Controls.cs b/Source/Controls.cs
index 49ff113..86b4b0d 100644
--- a/Source/Controls.cs
+++ b/Source/Controls.cs
@@ -26,11 +26,17 @@ public static class Controls
                Camera.Add(Keys.A, Keys.D, Keys.W, Keys.S);

                Jump.Clear();
-               Jump.Add(0, Buttons.A, Buttons.Y);
+               Jump.Add(0, gamepad => gamepad switch {
+                               Gamepads.Nintendo => [Buttons.B, Buttons.X],
+                               _ => [Buttons.A, Buttons.Y],
+                               });
                Jump.Add(Keys.C);

                Dash.Clear();
-               Dash.Add(0, Buttons.X, Buttons.B);
+               Dash.Add(0, gamepad => gamepad switch {
+                               Gamepads.Nintendo => [Buttons.Y, Buttons.A],
+                               _ => [Buttons.X, Buttons.B],
+                               });
                Dash.Add(Keys.X);

                Climb.Clear();
diff --git a/Framework/Input/VirtualButton.cs b/Framework/Input/VirtualButton.cs
index cc96e78..84fc360 100644
--- a/Framework/Input/VirtualButton.cs
+++ b/Framework/Input/VirtualButton.cs
@@ -35,6 +35,20 @@ public class VirtualButton
        public float ValueNoDeadzone => IsDown ? 1.0f : 0.0f;
    }

+   public record GamepadDependentButtonBinding(int Controller, Func<Gamepads, Buttons[]> Button) : IBinding
+   {
+       internal bool buttonCond(Func<Controller, Buttons, bool> cond) {
+           var controller = Input.Controllers[Controller];
+           var buttons = Button(controller.Gamepad);
+           return buttons.Any(button => cond(controller, button));
+       }
+       public bool IsPressed => buttonCond((controller, button) => controller.Pressed(button));
+       public bool IsDown => buttonCond((controller, button) => controller.Down(button));
+       public bool IsReleased => buttonCond((controller, button) => controller.Released(button));
+       public float Value => IsDown ? 1.0f : 0.0f;
+       public float ValueNoDeadzone => IsDown ? 1.0f : 0.0f;
+   }
+
    public record MouseBinding(MouseButtons Button) : IBinding
    {
        public bool IsPressed => Input.Mouse.Pressed(Button);
@@ -177,6 +191,12 @@ public class VirtualButton
        return this;
    }

+   public VirtualButton Add(int controller, Func<Gamepads, Buttons[]> buttons)
+   {
+       Bindings.Add(new GamepadDependentButtonBinding(controller, buttons));
+       return this;
+   }
+
    public VirtualButton Add(int controller, Axes axis, int sign, float threshold)
    {
        Bindings.Add(new AxisBinding(controller, axis, sign, threshold));
kwrotunni commented 5 months ago

Just wanted to note that, currently, when using a Switch Pro controller, it displays "Ⓑ Confirm Ⓐ Exit(/Back/Cancel)" on the title screen, but it's still using Nintendo!A (east) to confirm and Nintendo!B (south) to exit. Which, honestly, is the behavior I personally prefer (I grew up with Nintendo stuff, hence using a Switch Pro controller), but that still leaves the issue of it mislabeling the buttons.

NoelFB commented 5 months ago

This should all be fixed now!

afontenot commented 5 months ago

Can confirm that the button inputs appear to be correct (B is now accept + jump). On the other hand, the labels are still for the XBOX buttons (showing A to confirm).

Edit: testing against version 1ee558c

NoelFB commented 5 months ago

@afontenot Oops I forgot about the issue with 8BitDo controllers, going to make a new issue specifically for that.