Skip to content

Fixed a crash in the response curve axis filter and rewritten it to use integer math#10

Open
LunarEclipse363 wants to merge 3 commits intoxiota:mainfrom
LunarEclipse363:main
Open

Fixed a crash in the response curve axis filter and rewritten it to use integer math#10
LunarEclipse363 wants to merge 3 commits intoxiota:mainfrom
LunarEclipse363:main

Conversation

@LunarEclipse363
Copy link

@LunarEclipse363 LunarEclipse363 commented Feb 4, 2026

As the TODOs in the old code implied there were unhandled edge cases, this PR solves that.

Comment on lines +52 to +53
int bucket_size = max_norm / bucket_count;
int bucket_index = std::min(value_norm / bucket_size, bucket_count - 1);
Copy link
Owner

Choose a reason for hiding this comment

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

Potential division by zero.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm yes this relies on some assumptions that should be upheld at the config validation level, I'll figure out what exactly those are and how to solve that later and update the PR.

Comment on lines +57 to +62
int bucket_size_final;
if (bucket_index == bucket_count - 1) {
bucket_size_final = max_norm - (bucket_size * (bucket_count - 1));
} else {
bucket_size_final = bucket_size;
}
Copy link
Owner

Choose a reason for hiding this comment

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

Potential division by zero later.

@LunarEclipse363
Copy link
Author

If I wanted to write some unit tests for this, how do I go about that?

I see there's a test/ directory but I couldn't find any guidance on how to run or write tests.

@xiota
Copy link
Owner

xiota commented Feb 7, 2026

If I wanted to write some unit tests for this, how do I go about that?

I don't know. I didn't update or add the tests to meson.

Are you using this as a driver or remapper? I no longer use it for either, so not sure it's worth investing a lot of time into anymore.

As a driver, there are some kernel modules that work well enough.

For basic remapping, you can set SDL_GAMECONTROLLERCONFIG environment variable. Use sdl3 testcontroller to generate.

For advanced remapping, I started a separate project. aelkey It's a Lua module, so can do almost anything. Turn gamepad into air mouse, midi controller, etc. Also not limited to gamepads.

@LunarEclipse363
Copy link
Author

I'm using it to make an old game (a revival of Need for Speed: World) running under Wine accept a DS5 controller, so it needs to be an xinput device with the exact name an Xbox 360 controller would use, as presented by Wine. Otherwise the game won't even attempt to use it. I've tried SC-Controller but it only really works properly for the original steam controller, the DS5 support is somewhat incomplete.

xboxdrv works great overall but I wanted to use a custom response curve for the steering axis to have better control and that's how I ran into a crash with this filter.

Could make another PR to make the configs I came up with as examples, the DS5 config seems especially useful to have, and a config that changes the device name to exactly pretend to be an Xbox 360 controller may be helpful to someone too.

@xiota
Copy link
Owner

xiota commented Feb 7, 2026

make an old game (a revival of Need for Speed: World) running under Wine accept a DS5 controller

Pretty cool that this is still useful... I revised the deprecation notice in the readme, but will still consider PRs/fixes to keep it usable.

exact name an Xbox 360 controller
custom response curve

Aelkey should be able to do that, but probably more effort because you'd have to implement the remapping/curves/etc in Lua. Latency from Lua averages around 0.2 ms.

Could make another PR to make the configs I came up with as examples

I wouldn't mind merging a PR with example configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants