go-sdl2
go-sdl2 copied to clipboard
veandco
Rework buttons
I, hopefully, made the mouse buttons a little more clear to use. In SDL, the buttons themselves (counted 1, 2, 3, etc) seemingly aren't used (except to generate the button masks) while the button masks are returned and expected to be tested against.
This change adds a Button type that represents the 'counted' button, and changes ButtonStateMask to just ButtonMask. The former has a function Mask that returns the corresponding button mask, and the latter has a function Has that can be used to test (using the 'counted' buttons). The latter can also be tested against like a normal bitmask if the user doesn't want to use that function.
Hi @dusk125, thank you again for the PR! Could you provide an example on how this would make the buttons clearer to use?
Hey of course! Sorry for the delay. Previously, to test for a button, you had to do:
button := MouseButtonEvent{} // would actually get from PollEvent
if button&sdl.ButtonLMask() != 0 {...}
Someone could try to use the sdl.BUTTON_* constants for checking the button state and have unexpected behavior since the button state in an event is suppose to be a mask of all buttons pressed/released.
if button == sdl.BUTTON_LEFT {...} // this wouldn't work
This PR would allow for:
button := MouseButtonEvent{} // would get from PollEvent
if button.Has(sdl.ButtonLeft) {...}
or
if button&sdl.ButtonMaskLeft != 0 {...}
or
if button&sdl.ButtonLMask() != 0 {...} // for backwards-compatibility
Someone shouldn't be able to easily equate button and sdl.ButtonLeft (for example) because this PR would also make them separate types further attempting to distinguish between the button mask and the button constant.
Thanks for your consideration!
Thank you @dusk125 for the explanation!
I think I see now though I suppose MouseButtonEvent doesn't need it as it's always one of the five constants if I understand correctly? It would make sense for MouseMotionEvent but I wonder if the additional methods such as Has() are superfluous as the built-in bitwise operators are well-understood and quite powerful because it can check for multiple buttons at the same time (e.g. if event.State & (sdl.ButtonLMask() | sdl.ButtonRMask()) != 0 {}).
However I think that it's a great idea to convert the masks from functions into constants (e.g. sdl.ButtonLMask() to sdl.ButtonLMask) as I believe users would expect them to be constants. I also think we should keep the type and constant names as they are to maintain straightforward translation from SDL2 so it's easier to guess the Go names if you already have previous SDL2 knowledge and it's also easier to review. What do you think @dusk125?
@veeableful makes sense and sounds great! Let me know if this is what you had in mind!
Hi @dusk125, thanks for the changes! I think you can replace the button mask functions such as ButtonLMask() functions with constants of the same names so the additional constants like ButtonMaskLeft are not required.