Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic, type safe event handling with Go 1.18 #192

Closed
200sc opened this issue Feb 18, 2022 · 1 comment · Fixed by #196
Closed

Generic, type safe event handling with Go 1.18 #192

200sc opened this issue Feb 18, 2022 · 1 comment · Fixed by #196
Milestone

Comments

@200sc
Copy link
Contributor

200sc commented Feb 18, 2022

With the introduction of type parameters for types and functions in Go 1.18, oak's event package could be changed significantly.

Currently, to bind and trigger a binding, one needs to use the empty interface object:

package event

func example() {
    b := NewBus(nil)
    b.Bind(Enter, 0, func(c CID, i interface{}) int {
        e, ok := i.(EnterPayload) // this needs to be known, or more realistically, looked up 
        if !ok {
            // ??? realistically, this should never happen, unless this event was triggered by bad code
        }
        // use e 
        return 0 
    })
    // ok, compiles and runs correctly
    b.Trigger(Enter, EnterPayload{
        //...
    })
    // bad, compiles, but bindings will fail, and if bindings don't type check they'll panic 
    b.Trigger(Enter, 42)
}

With 1.18 we can do this:

package event 

type TypedEventID[T any] struct {
	EventID string 
}

type SafeBindable[T any] func(CID, T) int

func TriggerSafe[T any](ev TypedEventID[T], data T) {
	DefaultBus.Trigger(ev.EventID, data)
}

func TriggerSafeHandler[T any](b Handler, ev TypedEventID[T], data T) {
	b.Trigger(ev.EventID, data)
}

func BindSafe[T any](ev TypedEventID[T], c CID, fn SafeBindable[T]) {
	DefaultBus.Bind(ev.EventID, c, func(c CID, f interface{}) int {
		tf := f.(T)
		return fn(c, tf)
	})	
}

func BindSafeHandler[T any](b Handler, ev TypedEventID[T], c CID, fn SafeBindable[T]) {
	b.Bind(ev.EventID, c, func(c CID, f interface{}) int {
		tf := f.(T)
		// could still check ok here but could a bad type get in? 
		return fn(c, tf)
	})	
}

var (
	SafeEnter = TypedEventID[EnterPayload]{
		EventID: Enter,
	}
        //... could predeclare all events with their payloads as top level variables, both in engine and in game
)

func example() {
	h := NewBus(nil) 
	BindSafeHandler[EnterPayload](h, SafeEnter, 0, func(c CID, e EnterPayload) int {
	        // use e 
		return 0
	})
	// ok, compiles and has runtime guarantees
	TriggerSafeHandler[EnterPayload](h, SafeEnter, EnterPayload{})
	// ok, does not compile
	TriggerSafeHandler[EnterPayload](h, SafeEnter, 42)
}

... and granted, all the Safe naming in there is verbose, but it would be backwards compatible, and in Oak 4 could be overhauled to consume the existing names.

A notable downside-- methods cannot receive type parameters, so working with custom handlers demands passing those handlers in as arguments. The use of non-event handler interfaces is rare, and we don't have a way of enabling this at all, so I'm more or less OK with this trade off.

This would enable us to move all of our event documentation to strongly typed structures, removing empty interfaces from the apparent event interface. Granted we are still using them internally as events pipe around their payloads, a. eventually that interface could be hidden or recommended against using directly and b. maybe there's some more generic functions we could write to remove that as well (although I have not found a way so far).

@200sc
Copy link
Contributor Author

200sc commented Feb 18, 2022

Related to this, if the standard way of interacting with events would involve predeclaring them with compile time typing, we'd be more incentivized to move event names from strings to integers-- one can no longer concatenate events together, so the main argument for strings (which could have been replaced by bitflags anyway, maybe), goes away, and our event library would have improved efficiency.

As another thought, it may be appropriate to have two event libraries, if this new syntax becomes too difficult to use for prototyping / beginners, but I'm not sold either way on that yet.

@200sc 200sc linked a pull request Mar 12, 2022 that will close this issue
4 tasks
@200sc 200sc added this to the 4.0 milestone Mar 12, 2022
@200sc 200sc closed this as completed Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant