veandco / go-sdl2

SDL2 binding for Go
https://godoc.org/github.com/veandco/go-sdl2
BSD 3-Clause "New" or "Revised" License
2.21k stars 221 forks source link

SysWMEvent member 'msg' not accessible #410

Closed kroppt closed 5 years ago

kroppt commented 5 years ago

Presently, there is no way to access a part of SysWMEvent, which is necessary for winapi menu bars.

https://github.com/veandco/go-sdl2/blob/1bad6977f9fe18e8c4a47dd0934015b7fb319ae6/sdl/events.go#L865-L871

Notice how msg is unexported.

This is what type msg points to in the SDL2 source: https://github.com/veandco/go-sdl2/blob/1bad6977f9fe18e8c4a47dd0934015b7fb319ae6/.go-sdl2-libs/include/SDL2/SDL_syswm.h#L132-L186

veeableful commented 5 years ago

Hi @kroppt! I don't have access to Windows machine right now but could you try to apply this patch and see if it works? You would be able to access via SysWMEvent.Msg.Windows().

https://transfer.sh/ImoDD/0001-sdl-syswm-add-way-to-access-internals-of-SysWMmsg.patch

kroppt commented 5 years ago
$ go build
# github.com/veandco/go-sdl2/sdl
.\syswm_windows.go:10:7: could not determine kind of name for C.HWND
.\syswm_windows.go:13:9: could not determine kind of name for C.LPARAM
.\syswm_windows.go:11:6: could not determine kind of name for C.UINT
.\syswm_windows.go:12:9: could not determine kind of name for C.WPARAM

I hope I didn't do anything wrong!

kroppt commented 5 years ago

If it helps: HWND is HANDLE is void *, which I have been using as uintptr in go. LPARAM is LONG_PTR is long (32-bit) or int64 (64-bit), which should be int in go. UINT is unsigned int, which is uint32 in go. WPARAM is UINT_PTR is unsigned int (32-bit) or unsigned int64 (64-bit), which I guess would be a int in go. It's really just the sizes that matter I suppose. Alternatively, you can C include WinDef.h. Source: https://docs.microsoft.com/en-us/windows/desktop/WinProg/windows-data-types

veeableful commented 5 years ago

@kroppt Okay, Let me try the header file approach! Could you try this patch?

https://transfer.sh/ImoDD/0001-sdl-syswm-add-way-to-access-internals-of-SysWMmsg.patch

kroppt commented 5 years ago

That patch was the same, but adding the include was trivial. I successfully tested it and got the expected result upon menu bar interaction. Thank you. Side note, this looks clumsy.

if evt.Msg.Windows().Msg == 0x0111 {
    fmt.Printf("%v\n", evt.Msg.Windows())
}
veeableful commented 5 years ago

@kroppt Oops, sorry about that! Do you mean the API is not user-friendly?

kroppt commented 5 years ago

Yes, that is what I meant.

veeableful commented 5 years ago

@kroppt Ahh okay. Do you think it would have been better if SysWMmsg contained the members of WindowsMsg directly?

kroppt commented 5 years ago

The more I think about it, the more sense it makes to leave it how it is. It looks odd, but this is mainly a quirk of the Windows version of the SDL API, because it has a member called msg. For example, in C this would look like:

msg = evt.syswm.msg->msg.win.msg

I'm not sure which way would be best here, but I would try to make it consistent with other parts of the API, even if that means the Windows version looks quirky. The way you did it in the patch seems to be similar to how SysWMInfo is implemented.

veeableful commented 5 years ago

Alright then. I'll push it to the test branch and then master branch some time today. I'll keep the API and perhaps revisit this in the future!

veeableful commented 5 years ago

I have merged the changes into the master branch. Let me know if it still works for you 😃

kroppt commented 5 years ago

Yes, it worked using latest. Thank you.