EngoEngine/ecs

Idiomatic Go - biggest breaking change yet

EtienneBruines opened this issue · 4 comments

So, I was told by some people at the slack-chat, that we aren't using idiomatic Go. There may be collisions between the Type() string methods, and we're wasting performance by doing constant lookups.

They said: why don't you use reflect? Well, reflect is horrible so I don't want to use it.

But the idea is simple: get rid of the global []Entity, which stores every Entity. Instead, add the user-made entities to the appropriate systems,


Usage would be something like:

type MyEntity struct {
    BasicEntity
    SpaceComponent
    RenderComponent
}

func main() {
    // There'd be some kind of global list of systems, which contain every system, so we can loop over them
    var systems []BasicSystem

    // Each system we create, we add to that global list of systems
    RS := &RenderSystem{}
    systems = append(systems, RS)

    // Then we can define initialize our own custom-made entity struct
    e := &MyEntity{}

    // Add the appropriate component-references to the RenderSystem - this way you know for sure which components the render-systems requires
    RS.Add(&e.BasicEntity, &e.SpaceComponent, &e.RenderComponent)

    // And now we loop
    for _, sys := range systems {
        sys.Update(0.25)
    }
}

Upsides;

  • It's blazing-fast. The current Component function takes 91ns per lookup, the ComponentFast about 18ns, and this approach takes 0ns. For something that might happen thousands / millions of times a second, this is nice.
  • It's idiomatic Go, in the sense that it uses Go's type-safety. If it compiles, it'll most-likely run.

Downsides:

  • You need a reference to each system you want to use. Where to keep those references? What if we changed Scenes? They would have to start using other references.
  • It would break everything

This change is huge, but might also be worth it. I'd love a discussion on this, and maybe a fix for the downside? @paked, @everyone?

A possible fix for the downside, is doing this, when adding an entity:

// Create the entity
e := &MyEntity{}

// Loop over all systems. This way we loop only once, and add it to all systems we like
for _, system := range systems {
    switch sys := system.(type) {
        case *RenderSystem:
            sys.Add(&e.BasicEntity, &e.SpaceComponent, &e.RenderComponent)
        case *MouseSystem:
            sys.Add(&e.BasicEntity, &e.SpaceComponent, &e.MouseComponent)
    }
}

Just to illustrate, I re-wrote the AnimationSystem to work with new syntax:

Old / current:

type AnimationSystem struct {
    ecs.LinearSystem
}

func (a *AnimationSystem) New(*ecs.World) {}

func (*AnimationSystem) Type() string { return "AnimationSystem" }
func (*AnimationSystem) Pre()         {}
func (*AnimationSystem) Post()        {}

func (a *AnimationSystem) UpdateEntity(entity *ecs.Entity, dt float32) {
    var (
        ac *AnimationComponent
        r  *RenderComponent
        ok bool
    )

    if ac, ok = entity.ComponentFast(ac).(*AnimationComponent); !ok {
        return
    }
    if r, ok = entity.ComponentFast(r).(*RenderComponent); !ok {
        return
    }

    ac.change += dt
    if ac.change >= ac.Rate {
        ac.NextFrame()
        r.SetDrawable(ac.Cell())
    }
}


New:

type animationEntity struct {
    *ecs.BasicEntity
    *AnimationComponent
    *RenderComponent
}

type AnimationSystem struct {
    entities []animationSystemEntity
}

func (a *AnimationSystem) Add(e *ecs.BasicEntity, anim *AnimationComponent, rend *RenderComponent) {
    a.entities = append(a.entities, []animationEntity{e, anim, rend})
}

func (a *AnimationSystem) Remove(basic ecs.BasicEntity) {
    delete := -1
    for index, e := range a.entities {
        if e.ID() == basic.ID() {
            delete = index
            break
        }
    }
    if delete >= 0 {
        a.entities = append(a.entities[:delete], a.entities[delete+1:]...)
    }
}

func (a *AnimationSystem) Update(dt float32) {
    for _, e := range a.entities {
        e.AnimationComponent.change += dt
        if e.AnimationComponent.change >= e.AnimationComponent.Rate {
            e.AnimationComponent.NextFrame()
            e.RenderComponent.SetDrawable(e.AnimationComponent.Cell())
        }
    }
}

So this basically made the Pre() and Post() functions not-needed, but requires some copying the Add and Remove code (which makes sense, because this code is different when you use an array, then when you use a map, or you might use anything completely different)

paked commented

On the Gitter I have expressed that I would like to get this into master.

LETS DO IT!

^ Please, thank you 💃
v