From 863a2c47c4fe140ffeb414c7740439e3f30f9860 Mon Sep 17 00:00:00 2001 From: Ryan McClelland Date: Mon, 10 Aug 2026 22:52:07 -0700 Subject: [PATCH] libretro: fix the Wiimote gyroscope and make MotionPlus a device type Three defects kept the gyroscope from ever working The sign was destroyed before it arrived. SensorDevice published each gyro axis as one signed input, but ControlExpression clamps a control to >= 0 (ExpressionParser.cpp, "We clamp off the negative values here"), so half the travel was discarded. With Pitch Up bound to GyroX and Pitch Down to GyroX*-1, IMUGyroscope::GetRawState() returned controls[1] minus controls[0], which works out to -2*max(0, GyroX): half the signal gone, the rest doubled and inverted. Each axis is now a one-sided pair (GyroX+/GyroX- and so on), which is what the accelerometer beside it already did and what every upstream gyro backend does for this exact reason. The binding was also nested inside the accelerometer branch, so a frontend offering a gyroscope but no accelerometer bound neither. It is now a sibling. Shutdown() crossed the two sensors over, disabling the gyroscope when the accelerometer had been enabled and vice versa, and addressed port 0 every time regardless of which port it was tearing down. MotionPlus becomes per-port hardware rather than a core option, because one player having the dongle fitted while another does not is the ordinary case, and Dolphin already stores the setting per Wiimote. It doubles the device list rather than adding one entry: the dongle passes the port through, so every extension still plugs in, into the dongle instead of into the remote. wiimote_base_device() collapses the new ids back to their twins so every existing branch stays written against the five it already knew. --- Source/Core/DolphinLibretro/Input.cpp | 159 +++++++++++++++++++++----- Source/Core/DolphinLibretro/Input.h | 24 ++-- 2 files changed, 149 insertions(+), 34 deletions(-) diff --git a/Source/Core/DolphinLibretro/Input.cpp b/Source/Core/DolphinLibretro/Input.cpp index 1da73bbc7493..41c447bb74d9 100644 --- a/Source/Core/DolphinLibretro/Input.cpp +++ b/Source/Core/DolphinLibretro/Input.cpp @@ -52,6 +52,53 @@ #define RETRO_DEVICE_WIIMOTE_CC_PRO ((5 << 8) | RETRO_DEVICE_JOYPAD) #define RETRO_DEVICE_GC_ON_WII ((6 << 8) | RETRO_DEVICE_JOYPAD) #define RETRO_DEVICE_REAL_WIIMOTE ((6 << 8) | RETRO_DEVICE_NONE) +// MotionPlus is a dongle in the expansion port, not a mode of the remote, so it +// gets device ids of its own rather than a core option: it is per-PORT hardware, +// and one player having it fitted while another does not is the ordinary case. +// +// It doubles the list rather than adding one entry, because the dongle PASSES THE +// PORT THROUGH: every extension still plugs in, into the dongle rather than into +// the remote. So each remote above has a with-MotionPlus twin, sideways included +// (that one is a way of holding the thing, not something plugged into it). +#define RETRO_DEVICE_WIIMOTE_MP ((7 << 8) | RETRO_DEVICE_JOYPAD) +#define RETRO_DEVICE_WIIMOTE_MP_SW ((8 << 8) | RETRO_DEVICE_JOYPAD) +#define RETRO_DEVICE_WIIMOTE_MP_NC ((9 << 8) | RETRO_DEVICE_JOYPAD) +#define RETRO_DEVICE_WIIMOTE_MP_CC ((10 << 8) | RETRO_DEVICE_JOYPAD) +#define RETRO_DEVICE_WIIMOTE_MP_CC_PRO ((11 << 8) | RETRO_DEVICE_JOYPAD) + +/// The same remote with the dongle taken back off, or the id unchanged when it +/// never had one. +/// +/// MotionPlus changes nothing about the buttons, the extension or the IR. It +/// sits between the remote and whatever else is plugged in and reports rotation. +/// So everything downstream stays written against the five original ids, and this +/// collapses the dongle out first rather than every branch having to name ten. +static inline unsigned wiimote_base_device(unsigned device) +{ + switch (device) + { + case RETRO_DEVICE_WIIMOTE_MP: + return RETRO_DEVICE_WIIMOTE; + case RETRO_DEVICE_WIIMOTE_MP_SW: + return RETRO_DEVICE_WIIMOTE_SW; + case RETRO_DEVICE_WIIMOTE_MP_NC: + return RETRO_DEVICE_WIIMOTE_NC; + case RETRO_DEVICE_WIIMOTE_MP_CC: + return RETRO_DEVICE_WIIMOTE_CC; + case RETRO_DEVICE_WIIMOTE_MP_CC_PRO: + return RETRO_DEVICE_WIIMOTE_CC_PRO; + default: + return device; + } +} + +/// True when this device id carries a MotionPlus dongle. Defined as "the id +/// changes when the dongle is removed", so the two can never disagree about +/// which ids are twins. +static inline bool wiimote_has_motion_plus(unsigned device) +{ + return wiimote_base_device(device) != device; +} typedef enum { SENSOR_ACCELEROMETER = 0, @@ -77,7 +124,8 @@ static bool sensor_enabled[NUM_CONTROLLERS_FOR_SENSORS][SENSOR_COUNT] = {}; static int port_max; double g_accel_pos[NUM_CONTROLLERS_FOR_SENSORS][3] = {}; // x, y, z double g_accel_neg[NUM_CONTROLLERS_FOR_SENSORS][3] = {}; // x, y, z -double g_gyro[NUM_CONTROLLERS_FOR_SENSORS][3] = {}; +double g_gyro_pos[NUM_CONTROLLERS_FOR_SENSORS][3] = {}; // x, y, z +double g_gyro_neg[NUM_CONTROLLERS_FOR_SENSORS][3] = {}; // x, y, z static struct retro_input_descriptor descGC[] = { {0, RETRO_DEVICE_JOYPAD, 0, RETRO_DEVICE_ID_JOYPAD_LEFT, "Left"}, @@ -529,6 +577,11 @@ void InitStage2() {"WiiMote + Nunchuk", RETRO_DEVICE_WIIMOTE_NC}, {"WiiMote + Classic Controller", RETRO_DEVICE_WIIMOTE_CC}, {"WiiMote + Classic Controller Pro", RETRO_DEVICE_WIIMOTE_CC_PRO}, + {"WiiMote + MotionPlus", RETRO_DEVICE_WIIMOTE_MP}, + {"WiiMote + MotionPlus (sideways)", RETRO_DEVICE_WIIMOTE_MP_SW}, + {"WiiMote + MotionPlus + Nunchuk", RETRO_DEVICE_WIIMOTE_MP_NC}, + {"WiiMote + MotionPlus + Classic Controller", RETRO_DEVICE_WIIMOTE_MP_CC}, + {"WiiMote + MotionPlus + Classic Controller Pro", RETRO_DEVICE_WIIMOTE_MP_CC_PRO}, {"Real WiiMote", RETRO_DEVICE_REAL_WIIMOTE}, }; @@ -555,6 +608,11 @@ void InitStage2() {"WiiMote + Nunchuk", RETRO_DEVICE_WIIMOTE_NC}, {"WiiMote + Classic Controller", RETRO_DEVICE_WIIMOTE_CC}, {"WiiMote + Classic Controller Pro", RETRO_DEVICE_WIIMOTE_CC_PRO}, + {"WiiMote + MotionPlus", RETRO_DEVICE_WIIMOTE_MP}, + {"WiiMote + MotionPlus (sideways)", RETRO_DEVICE_WIIMOTE_MP_SW}, + {"WiiMote + MotionPlus + Nunchuk", RETRO_DEVICE_WIIMOTE_MP_NC}, + {"WiiMote + MotionPlus + Classic Controller", RETRO_DEVICE_WIIMOTE_MP_CC}, + {"WiiMote + MotionPlus + Classic Controller Pro", RETRO_DEVICE_WIIMOTE_MP_CC_PRO}, {"Real WiiMote", RETRO_DEVICE_REAL_WIIMOTE}, {"GameCube Controller", RETRO_DEVICE_GC_ON_WII}, }; @@ -643,11 +701,14 @@ void Shutdown() { Pad::ResetRumble(i); - if(sensor_enabled[i][SENSOR_ACCELEROMETER]) - sensor_interface.set_sensor_state(0, RETRO_SENSOR_GYROSCOPE_DISABLE, 0); + // Each sensor turns off the one it actually turned on, on the port it was + // enabled for. This used to cross the two over and address port 0 every + // time, so a multi-remote session left every sensor but port 0's running. + if (sensor_enabled[i][SENSOR_ACCELEROMETER]) + sensor_interface.set_sensor_state(i, RETRO_SENSOR_ACCELEROMETER_DISABLE, 0); - if(sensor_enabled[i][SENSOR_GYRO]) - sensor_interface.set_sensor_state(0, RETRO_SENSOR_ACCELEROMETER_DISABLE, 0); + if (sensor_enabled[i][SENSOR_GYRO]) + sensor_interface.set_sensor_state(i, RETRO_SENSOR_GYROSCOPE_DISABLE, 0); sensor_enabled[i][SENSOR_ACCELEROMETER] = false; sensor_enabled[i][SENSOR_GYRO] = false; @@ -672,7 +733,10 @@ void UpdateAccelerometer(unsigned port) float ay = sensor_interface.get_sensor_input(port, RETRO_SENSOR_ACCELEROMETER_Y) * G; float az = sensor_interface.get_sensor_input(port, RETRO_SENSOR_ACCELEROMETER_Z) * G; - if (input_types[port] == RETRO_DEVICE_WIIMOTE_SW) + // Collapsed, so a sideways remote with the dongle fitted still turns: holding + // it sideways rotates what its sensors read, and MotionPlus does not change + // which way up it is being held. + if (wiimote_base_device(input_types[port]) == RETRO_DEVICE_WIIMOTE_SW) { float rx = -ay; // rotate 90° clockwise float ry = ax; @@ -696,11 +760,13 @@ void UpdateGyro(unsigned port) if (!sensor_enabled[port][SENSOR_GYRO] || !sensor_interface.get_sensor_input) return; + // Angular velocity in rad/s about the remote's own axes, same frame the + // accelerometer above arrives in: +X left, +Y back, +Z up. float gx = sensor_interface.get_sensor_input(port, RETRO_SENSOR_GYROSCOPE_X); float gy = sensor_interface.get_sensor_input(port, RETRO_SENSOR_GYROSCOPE_Y); float gz = sensor_interface.get_sensor_input(port, RETRO_SENSOR_GYROSCOPE_Z); - if (input_types[port] == RETRO_DEVICE_WIIMOTE_SW) + if (wiimote_base_device(input_types[port]) == RETRO_DEVICE_WIIMOTE_SW) { float rx = -gy; // rotate 90° clockwise float ry = gx; @@ -708,9 +774,17 @@ void UpdateGyro(unsigned port) gy = ry; } - g_gyro[port][0] = gx; - g_gyro[port][1] = gy; - g_gyro[port][2] = gz; + // Split each axis across a one-sided pair. A control cannot carry a negative + // value through the expression parser (see SensorDevice::RegisterAll), so the + // sign lives in WHICH slot is non-zero rather than in the number itself. + g_gyro_pos[port][0] = std::max(0.0f, gx); + g_gyro_neg[port][0] = std::max(0.0f, -gx); + + g_gyro_pos[port][1] = std::max(0.0f, gy); + g_gyro_neg[port][1] = std::max(0.0f, -gy); + + g_gyro_pos[port][2] = std::max(0.0f, gz); + g_gyro_neg[port][2] = std::max(0.0f, -gz); } void ResetControllers(const WiimoteUpdateFlags& f) @@ -814,6 +888,10 @@ static std::string GetQualifiedNameSensor(unsigned port) // can be called from retro_run, do not reset all settings because one thing changed void UpdateWiimoteMappings(const WiimoteUpdateFlags& f, unsigned port, unsigned device) { + // Nothing below is affected by the dongle, and ResetControllers hands us + // input_types[port] verbatim, which may name a MotionPlus variant. + device = wiimote_base_device(device); + if (!f.any() || device == RETRO_DEVICE_REAL_WIIMOTE || device == RETRO_DEVICE_WIIMOTE_CC || device == RETRO_DEVICE_WIIMOTE_CC_PRO) return; @@ -1073,7 +1151,11 @@ void retro_set_controller_port_device(unsigned port, unsigned device) { retro_input_descriptor* desc; - switch (Libretro::Input::input_types[i]) + // Through the same collapse as everywhere else: the dongle adds no buttons, + // so a MotionPlus remote wants its twin's descriptors. Left raw, every + // MotionPlus id would fall to default and a Nunchuk's labels would go + // missing. + switch (wiimote_base_device(Libretro::Input::input_types[i])) { case RETRO_DEVICE_WIIMOTE_SW: desc = Libretro::Input::descWiimoteSideways; @@ -1323,6 +1405,12 @@ void retro_set_controller_port_device_wii(unsigned port, unsigned device) #endif auto& si = Core::System::GetInstance().GetSerialInterface(); + // Take the dongle off the id and remember it separately, so every branch below + // only ever sees the four remotes it was written for. The flag is applied once, + // beside the extension selection it belongs with. + const bool wantMotionPlus = wiimote_has_motion_plus(device); + device = wiimote_base_device(device); + if (Wiimote::GetConfig()->ControllersNeedToBeCreated()) { WARN_LOG_FMT(COMMON, "No controllers have been created yet"); @@ -1476,23 +1564,31 @@ void retro_set_controller_port_device_wii(unsigned port, unsigned device) wmAccel->SetControlExpression(4, "`" + devSensor + ":AccelY-`"); // Forward wmAccel->SetControlExpression(5, "`" + devSensor + ":AccelY+`"); // Backward } + } - if (Libretro::Input::sensor_enabled[port][SENSOR_GYRO]) + // A sibling of the accelerometer branch, not a child of it. Nested, a + // frontend that offered gyro but no accelerometer bound neither. + if (Libretro::Input::sensor_enabled[port][SENSOR_GYRO]) + { + // Gyroscope (6 inputs: PitchUp/Down, RollLeft/Right, YawLeft/Right) + auto* wmGyro = static_cast( + wm->GetWiimoteGroup(WiimoteEmu::WiimoteGroup::IMUGyroscope)); + if (wmGyro) { - // Gyroscope (6 inputs: PitchUp/Down, RollLeft/Right, YawLeft/Right) - auto* wmGyro = static_cast( - wm->GetWiimoteGroup(WiimoteEmu::WiimoteGroup::IMUGyroscope)); - if (wmGyro) - { - // Map libretro axes to Wiimote angular axes: - // Pitch ~ rotation around X, Roll ~ rotation around Y, Yaw ~ rotation around Z - wmGyro->SetControlExpression(0, "`" + devSensor + ":GyroX`"); // Pitch Up - wmGyro->SetControlExpression(1, "`" + devSensor + ":GyroX`*-1"); // Pitch Down - wmGyro->SetControlExpression(2, "`" + devSensor + ":GyroY`*-1"); // Roll Left - wmGyro->SetControlExpression(3, "`" + devSensor + ":GyroY`"); // Roll Right - wmGyro->SetControlExpression(4, "`" + devSensor + ":GyroZ`*-1"); // Yaw Left - wmGyro->SetControlExpression(5, "`" + devSensor + ":GyroZ`"); // Yaw Right - } + // Angular velocity about the remote's own axes, the same frame the + // accelerometer uses: +X left, +Y back, +Z up, right-hand rule. That + // fixes which direction each axis names: about +X the nose drops, so + // +X is pitch DOWN; about +Y the top rolls left; about +Z the nose + // swings left. + // + // GetRawState() reads these as [1]-[0], [2]-[3], [4]-[5], so pairing + // them this way hands back exactly the signed value that arrived. + wmGyro->SetControlExpression(0, "`" + devSensor + ":GyroX-`"); // Pitch Up + wmGyro->SetControlExpression(1, "`" + devSensor + ":GyroX+`"); // Pitch Down + wmGyro->SetControlExpression(2, "`" + devSensor + ":GyroY+`"); // Roll Left + wmGyro->SetControlExpression(3, "`" + devSensor + ":GyroY-`"); // Roll Right + wmGyro->SetControlExpression(4, "`" + devSensor + ":GyroZ+`"); // Yaw Left + wmGyro->SetControlExpression(5, "`" + devSensor + ":GyroZ-`"); // Yaw Right } } } @@ -1527,6 +1623,17 @@ void retro_set_controller_port_device_wii(unsigned port, unsigned device) ControllerEmu::Attachments* wmExtension = (ControllerEmu::Attachments*)wm->GetWiimoteGroup(WiimoteGroup::Attachments); + // Fit or remove the dongle. Index 0 is "Attach MotionPlus", the only entry in + // this group's numeric_settings. The attachment SELECTOR is deliberately kept + // out of that list (Attachments.h), so it cannot be what gets written here. + // + // Dolphin defaults this to true for every remote, so it has to be written on + // BOTH paths rather than only when the dongle is wanted: a port re-announced + // without one would otherwise keep whatever the last remote left behind. + if (!wmExtension->numeric_settings.empty()) + static_cast*>(wmExtension->numeric_settings[0].get()) + ->SetValue(wantMotionPlus); + static_cast*>(wmOptions->numeric_settings[0].get()) ->SetValue(0); // Speaker Pan [-100, 100] static_cast*>(wmOptions->numeric_settings[1].get()) diff --git a/Source/Core/DolphinLibretro/Input.h b/Source/Core/DolphinLibretro/Input.h index 895d209ba7ea..8a23c6e2e0ad 100644 --- a/Source/Core/DolphinLibretro/Input.h +++ b/Source/Core/DolphinLibretro/Input.h @@ -37,7 +37,8 @@ namespace Input constexpr std::string_view source = "Libretro"; extern double g_accel_pos[NUM_CONTROLLERS_FOR_SENSORS][3]; extern double g_accel_neg[NUM_CONTROLLERS_FOR_SENSORS][3]; -extern double g_gyro[NUM_CONTROLLERS_FOR_SENSORS][3]; +extern double g_gyro_pos[NUM_CONTROLLERS_FOR_SENSORS][3]; +extern double g_gyro_neg[NUM_CONTROLLERS_FOR_SENSORS][3]; static retro_sensor_interface sensor_interface = {0}; @@ -78,11 +79,21 @@ class SensorDevice : public ciface::Core::Device }; public: + /// Every axis is published as a PAIR of one-sided inputs, never as one signed + /// input. ControlExpression::GetValueIgnoringSuppression() clamps a control to + /// >= 0 (ExpressionParser.cpp: "We clamp off the negative values here"), so a + /// lone signed input loses half its travel outright. The IMU groups are built + /// for exactly this shape, each one subtracting one direction's control from + /// the other's, so handing them a matched +/- pair reconstructs the signed + /// value the sensor actually reported. void RegisterAll() { - AddInput(new ScalarInput("GyroX", &Libretro::Input::g_gyro[m_port][0])); - AddInput(new ScalarInput("GyroY", &Libretro::Input::g_gyro[m_port][1])); - AddInput(new ScalarInput("GyroZ", &Libretro::Input::g_gyro[m_port][2])); + AddInput(new ScalarInput("GyroX+", &Libretro::Input::g_gyro_pos[m_port][0])); + AddInput(new ScalarInput("GyroX-", &Libretro::Input::g_gyro_neg[m_port][0])); + AddInput(new ScalarInput("GyroY+", &Libretro::Input::g_gyro_pos[m_port][1])); + AddInput(new ScalarInput("GyroY-", &Libretro::Input::g_gyro_neg[m_port][1])); + AddInput(new ScalarInput("GyroZ+", &Libretro::Input::g_gyro_pos[m_port][2])); + AddInput(new ScalarInput("GyroZ-", &Libretro::Input::g_gyro_neg[m_port][2])); AddInput(new ScalarInput("AccelX+", &Libretro::Input::g_accel_pos[m_port][0])); AddInput(new ScalarInput("AccelX-", &Libretro::Input::g_accel_neg[m_port][0])); AddInput(new ScalarInput("AccelY+", &Libretro::Input::g_accel_pos[m_port][1])); @@ -132,7 +143,7 @@ class GyroDevice : public ciface::Core::Device }; public: - GyroDevice(unsigned port) : m_port(port) + GyroDevice(unsigned port) { AddInput(new GyroAxis(port, GyroAxis::PITCH, "Pitch")); AddInput(new GyroAxis(port, GyroAxis::ROLL, "Roll")); @@ -141,7 +152,4 @@ class GyroDevice : public ciface::Core::Device std::string GetName() const override { return "Gyroscope"; } std::string GetSource() const override { return std::string(Libretro::Input::source); } - -private: - unsigned m_port; };