Fix SdlGamepad/SdlJoystick state going stale with no event subscribers - #2605
Merged
Merged
Conversation
…bers The array assignment (_buttons[i] = new Button(...), _triggers[i] = new Trigger(...)) was written inside the argument list of the ButtonDown/ButtonUp/TriggerMoved ?.Invoke(...) call. C#'s null-conditional operator short-circuits the whole call, including evaluating its arguments, when the event has no subscribers. So with nobody subscribed, the internal arrays never actually got updated, and polling Buttons/Triggers read permanently stale state even with real button presses. Moves the assignment to its own statement before the event fires, so it always runs regardless of subscribers. See issue dotnet#2604 for the full writeup and repro.
SdlJoystick.cs had the exact same bug for AxisMoved, HatMoved, ButtonDown, and ButtonUp: the array assignment was inside the ?.Invoke(...) argument list, so it never ran with zero subscribers. Same fix, moved to its own statement before the event fires. Also dropped the local variables from the SdlGamepad fix in favor of just re-reading the array (matches how ThumbstickMoved, the one case that was already correct, does it).
vchelaru
changed the title
Contributor
Author
|
@dotnet-policy-service agree [company="{your company}"] @dotnet-policy-service agree |
github-project-automation
Bot
moved this from Todo
to In Progress
in Silk.NET Development Tracker
Perksey
enabled auto-merge (squash)
July 27, 2026 04:18
github-project-automation
Bot
moved this from In Progress
to Done
in Silk.NET Development Tracker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2604
SdlGamepad and SdlJoystick's button/axis/trigger/hat handlers wrote the internal state array update as a side effect inside the argument list of the
?.Invoke(...)call for their change events (ButtonDown,ButtonUp,TriggerMovedon SdlGamepad;AxisMoved,HatMoved,ButtonDown,ButtonUpon SdlJoystick). C#'s null-conditional operator short-circuits the whole call, including evaluating the arguments, when the event has no subscribers. So if nobody's subscribed to those events, the array never actually gets updated, and polling the corresponding state (Buttons,Triggers,Axes,Hats) reads permanently stale data even with real input on the device.ThumbstickMoved on SdlGamepad already did this correctly (array assignment as its own statement before the event fires), so this brings the rest in line with that same shape.
Full writeup, repro steps, and confirmation this affects the latest release/main (but not GlfwGamepad, which rewrites state unconditionally every frame) are in #2604.