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

Idiomatic Go - biggest breaking change yet #13

Closed
EtienneBruines opened this issue Apr 11, 2016 · 4 comments
Closed

Idiomatic Go - biggest breaking change yet #13

EtienneBruines opened this issue Apr 11, 2016 · 4 comments

Comments

@EtienneBruines
Copy link
Member

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?

@EtienneBruines
Copy link
Member Author

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)
    }
}

@EtienneBruines
Copy link
Member Author

EtienneBruines commented Apr 16, 2016

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
Copy link
Member

paked commented Apr 19, 2016

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

LETS DO IT!

@Newbrict
Copy link

^ Please, thank you 💃
v

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants