oakmound/oak

Bindings should take in event.CID instead of int

Closed this issue · 4 comments

recently I got this error.
cannot use -math.Abs(p.HeldObj.Solid.Space.Label) (type float64) as type collision.Label in argument to p.HeldObj.Solid.UpdateLabel
this one was my fault, but my other point still stands.
We should either use a custom type or not, not sometimes use a custom type and sometimes not.
I've also had a similar thing happen with CIDs (GetEntity takes int).

200sc commented

The need to convert between int and event.CID in places been a longstanding annoyance, yes. I think it came from the place of wanting bindings to be quicker to write (func(int,interface{})int as better than func(event.CID,interface{})int-- but we can write some helpers to make some things better with the length of the function, and its not a significant difference.

But go will automatically convert number literals to custom types?
(Example)
Even within oak, the platforming tutorial uses ints as a cid in entities.NewMoving, which takes a cid value.

200sc commented

To be clear, I agree we should change bindings and functions like GetEntity to use event.CID. I was attempting to explain the original source of not doing that, and that was that most bindings have a start like this:

entity.Bind(func(id int, payload interface{}) int {
    ent, ok := event.GetEntity(id).(thisType)
    // ...
    return 0
}, eventName)

And we were concerned about the number of bindings people would have to type and how intuitive / long it would be to write event.CID instead of int for the first argument. I don't think that's a valid justification now, I just wanted to give context.

Ah, I see.