No way to get CommonEvent

[mbenkmann]

Unless I'm missing something there does not seem to be any way to obtain a CommonEvent from an Event without using direct pointer conversion with package "unsafe". Some way should be added to either get a *CommonEvent or to otherwise access Timestamp and Type from an sdl.Event.

[malashin]

Hello @mbenkmann

Can you please give an example of what you want to achieve and how?
I'm not sure why you would need a CommonEvent and how you are supposed to receive it.

All events have Timestamp and Type exported.

CommonEvent is returned as default value in event type switch:
https://github.com/veandco/go-sdl2/blob/master/sdl/events.go#L644

func goEvent(cevent *CEvent) Event {
	switch cevent.Type {
	case WINDOWEVENT:
		return (*WindowEvent)(unsafe.Pointer(cevent))
	...
	default:
		return (*CommonEvent)(unsafe.Pointer(cevent))
	}
}

But it's probably never returned since SDL events usually have more specific types.

[mbenkmann]

What I want to do is use the Timestamp in my event loop, so that I can process events for the proper frames (because of CPU load and process scheduling it's possible that I miss frame and have to catch up).

for event := sdl.PollEvent(); event != nil; event = sdl.PollEvent() {
  ts := event.Timestamp  // Does not work, neither does event.(*CommonEvent).Timestamp
  if ts < current_frame_start_time {
      fix_previous_frame(event)
  }      
}

The marked line could be something like

ts := sdl.GetTimestamp(event)

or

ts := sdl.GetCommon(event).Timestamp // GetCommon() returns *CommonEvent

Both functions are trivial casts using unsafe.Pointer, but I would like that ugliness to be in the library code not in my client code.

[mbenkmann]

Another use case I have is a function that has a type switch with common code using the timestamp for all cases. Right now it looks like this:

var timestamp uint32
 switch ev := ev.(type) {
  case *sdl.JoyButtonEvent:
      timestamp = ev.Timestamp
      ...
   case *sdl.JoyHatEvent:
      timestamp = ev.Timestamp
   ...
}

// common code using timestamp

It's really stupid to have to start every case with

timestamp = ev.Timestamp

Again, it would be way better to have one of the functions I detailed above.

[mbenkmann]

Even nicer of course would be to have sdl.Event give access to the fields directly. So the subtypes would have to be declared like this

// this assumes that Event is redefined to be what CommonEvent is now, i.e.
// the common part with the Timestamp and Type fields

type JoyDeviceEvent struct {
    Event
    Which     JoystickID 
}

NOTE: I am not an expert on the implementation details of Go, so I do not know if the memory layout of this composite struct is guaranteed to be compatible with C. I think it's very likely but I would double-check if you go this route.

[malashin]

This is another issue of Go not having unions.

sdl.Event is not a struct, it's an interface.
Since all event types implement it (because it's empty) type assertion is used to access events data directly.

The easy solution would be adding methods to access Type and Timestamp that are common to all event types.

// Event is a union of all event structures used in SDL.
// (https://wiki.libsdl.org/SDL_Event)
type Event interface {
	GetType() uint32      // the event type
	GetTimestamp() uint32 // timestamp of the event
}

// CommonEvent contains common event data.
// (https://wiki.libsdl.org/SDL_Event)
type CommonEvent struct {
	Type      uint32 // the event type
	Timestamp uint32 // timestamp of the event
}

// GetType returns the event type.
func (e *CommonEvent) GetType() uint32 {
	return e.Type
}

// GetTimestamp returns timestamp of the event.
func (e *CommonEvent) GetTimestamp() uint32 {
	return e.Timestamp
}

After doing this to all event types it will be possible to receive the needed data before the type assertion:

for event := sdl.PollEvent(); event != nil; event = sdl.PollEvent() {
	fmt.Println(event.GetTimestamp())
}

Doing it this way won't add any breaking changes to the library. But I do not like the duplication of both exporting the struct data and creating a getter method for it, also "Get" should not be used in method names in Go, and we can't have both exported struct field and method have the same names.

I wonder if anyone does changes to the events data. If not, it would be nicer to have all event structs data received with getter methods and unexported in the structs themselves.

// JoyDeviceEvent contains joystick device event information.
// (https://wiki.libsdl.org/SDL_JoyDeviceEvent)
type JoyDeviceEvent struct {
	_type     uint32     // JOYDEVICEADDED, JOYDEVICEREMOVED
	timestamp uint32     // the timestamp of the event
	which     JoystickID // the joystick device index for the JOYDEVICEADDED event or the instance id for the JOYDEVICEREMOVED event
}

// Type returns the event type.
func (e *JoyDeviceEvent ) Type() uint32 {
	return e._type
}

// Timestamp returns timestamp of the event.
func (e *JoyDeviceEvent ) Timestamp() uint32 {
	return e.timestamp
}

// Timestamp returns timestamp of the event.
func (e *JoyDeviceEvent ) Which() uint32 {
	return e.which
}

On other hand this is a breaking change and everyone would need to rewrite their event loops from using exported struct field to methods.

[veeableful]

Creating functions for getting the type and timestamp seems to require less work for both sides. If it's not a method, I believe it doesn't violate the Go idiom of not having Get in a function name? Perhaps in the future, we can make the breaking change if really necessary.

[mbenkmann]

"I wonder if anyone does changes to the events data."

Please do not make the struct fields private. Being able to create and modify instances of sdl structures in client code is crucial for writing unit tests.

" would be nicer to have [...] getter methods [..]"

puke. Maybe for a Java programmer. They've spread around this disease.

[mbenkmann]

I've created 3 possible solutions for this issue. The following are the implementation links, followed by what the client code would look like, assuming

var event sdl.Event

https://github.com/mbenkmann/go-sdl2/commit/71e00c91d82e965f5a68221855b98faff496b15c

ts := sdl.Common(event).Timestamp
typ := sdl.Common(event).Type

https://github.com/mbenkmann/go-sdl2/commit/3a4151d407cf6f8418d1339adcf8b4c335ea8faf

ts := sdl.TimestampOf(event)
typ := sdl.TypeOf(event)

https://github.com/mbenkmann/go-sdl2/commit/1b31011754d367d5c5d9672994055cf773c41d2f

// Alternative 1: The Get prefix is necessary and there's nothing wrong with that.
ts := event.GetTimestamp() 
typ := event.GetType()

// Alternative 2    
ts := event.Common().Timestamp
typ := event.Common().Type

[malashin]

Added two new methods to event interface. No breaking changes.

// Event is a union of all event structures used in SDL.
// (https://wiki.libsdl.org/SDL_Event)
type Event interface {
	GetType() uint32      // the event type
	GetTimestamp() uint32 // timestamp of the event
}