Odin
Odin copied to clipboard
Proposal: Make `vendor:glfw` more idiomatic Odin
For the most part the GLFW bindings are a direct mapping of the original source files. For instance lots of #defines are mapped to constants in Odin.
I think there is room for improvement here. Adding more explicit type definitions would make the code using GLFW more idiomatic Odin code and catch bugs during compilation. I am mainly talking about grouping the long list of constants into sensible enums and adjusting the procedure definitions accordingly.
I made some initial tests and would be willing to go over the complete GLFW bindings and convert everything to contribute to the Odin project. However, I wanted to see if there are reasons against making a change like this before putting in all the effort.
See below for some examples.
Please let me know if you would be open to the proposed changes or if there is a reason against it, that I am not aware of. Thanks! :)
Example 1: GetKey
The constants are already grouped by comments in the source code (both original GLFW and vendor:glfw). So for instance the key states
https://github.com/odin-lang/Odin/blob/8135dda2fc42b3c474f0a77d7f03126b2eb8c034/vendor/glfw/constants.odin#L16-L19
could be grouped as
KeyState :: enum i32 {
RELEASE = 0,
PRESS = 1,
REPEAT = 2,
}
Introducing the KeyState enum from above and a Keys enum for all the key constants (left out for brevity) the definition of GetKey could be changed from
https://github.com/odin-lang/Odin/blob/d4a1670b93a174056b309db7e0e851e5e516bccb/vendor/glfw/bindings/bindings.odin#L116
to
GetKey :: proc(window: WindowHandle, key: Keys) -> KeyState ---
The usage code would change from
// before
if glfw.GetKey(window, glfw.KEY_SPACE) == glfw.PRESS {
fmt.println("space is pressed")
}
to
// after
if glfw.GetKey(window, .KEY_SPACE) == .PRESS {
fmt.println("space is pressed")
}
Example 2: SetInputMode
Turning
https://github.com/odin-lang/Odin/blob/8135dda2fc42b3c474f0a77d7f03126b2eb8c034/vendor/glfw/constants.odin#L346-L356
into enums InputMode and CursorMode
/* Cursor draw state and whether keys are sticky */
InputMode :: enum i32 {
CURSOR = 0x00033001,
STICKY_KEYS = 0x00033002,
STICKY_MOUSE_BUTTONS = 0x00033003,
LOCK_KEY_MODS = 0x00033004,
}
/* Cursor draw state */
CursorMode :: enum i32 {
CURSOR_NORMAL = 0x00034001,
CURSOR_HIDDEN = 0x00034002,
CURSOR_DISABLED = 0x00034003,
CURSOR_CAPTURED = 0x00034004,
}
would allow the definition of SetInputMode to be changed from
https://github.com/odin-lang/Odin/blob/d4a1670b93a174056b309db7e0e851e5e516bccb/vendor/glfw/bindings/bindings.odin#L148
to the more expressive
SetInputMode :: proc (window: WindowHandle, mode: InputMode, value: union { CursorMode, b32 }) ---
The usage code would change from
// before
glfw.SetInputMode(window, glfw.CURSOR, glfw.CURSOR_NORMAL)
glfw.SetInputMode(window, glfw.STICKY_KEYS, 1) // See below
to
// after
glfw.SetInputMode(window, .CURSOR, .CURSOR_NORMAL)
glfw.SetInputMode(window, .STICKY_KEYS, true)
Bonus fix regarding the 1 above: Using glfw.TRUE (like in C) here does not work as glfw.TRUE is defined as true in Odin, but SetInputMode expects an i32 here. After my proposed change true (or glfw.TRUE if you like) can be used here.