kevinresol / exp-ecs

Experimental macro-powered Entity-Component-System game engine
59 stars 5 forks source link

@:nodes causes errors when using type inference #5

Open Jarrio opened 5 years ago

Jarrio commented 5 years ago

Reproducible:

Main.hx

package;

import haxe.Timer;
import exp.ecs.Engine;

class Main {
    static function main() {
        var engine = new Engine();
        engine.systems.add(new MovementSystem());
        // run the engine
        new Timer(16).run = function() engine.update(16 / 1000);
    }
}

MovementSystem.hx

package;
import exp.ecs.component.Component;
import Main;
import exp.ecs.node.Node;
import tink.CoreApi.Noise;
import exp.ecs.system.System;

class MovementSystem extends System<Noise> {
    // prepares a NodeList that contains entities having both the Position and Velocity components
    @:nodes var nodes:Node<Velocity<Int>>;

    override function update(dt:Float) {
        // on each update we iterate all the nodes and update their Position components
        for(node in nodes) {
            // node.velocity.y +=  dt;
        }
    }
}

class Velocity<Type> extends Component {
    public function new(test:Type):Void {

    }
}```

Error:

Message: src/MovementSystem.hx:9: characters 20-34 : Expected a class that extends Component, but Velocity doesn't.```

kevinresol commented 5 years ago

I don't think type parameter on components would be supported in the current implementation. For one you can't run entity.get(Velocity<Int>) to get the component. Because at runtime you can't distinguish different type parameters.

Perhaps I can make entity.get a compile time method. Or we need some other mechanism to reference a component.

Also perhaps we can have some better error message for your problem.

@Antriel any comments?

Jarrio commented 5 years ago

I thought a typedef might have been able to be a good workaround but that also failed

typedef Foo = Velocity<Int>
kevinresol commented 5 years ago

As I said you can't distinguish the type parameters at run time. So that entity.get() will not work. And because of that, at lot of things won't work downstream, including nodelist.

Antriel commented 5 years ago

As for generics, I don't have much idea about how they are actually implemented, so I'm not sure if/how it could work. When using @:generic it should actually build up extra classes for each type, right? So then it might work, probably?

As for ECS, I don't see anything wrong with generics, but also not much practical use for them (at least so far, in my cases). I.e. while there's probably nothing inherently wrong with such usage, if it's too much trouble to implement, better not add it. Definitely not if it would sacrifice performance in any way.

As for changing how entity.get is implemented, I personally value performance and flexibility (in that order). Right now it's a hash map on entity instance I think, which isn't really the performant-way implementation. But since we aren't in low level language, I'm not sure what else we could do. Proper ECS libraries usually use sparse arrays (so, just arrays for us? at least on js it should work well), to map entity Id into a component. Whether it would actually improve performance is difficult to say. It would probably speed up random queries for certain components (i.e. something dynamic without prepared @:nodes array).

But thinking about it, as far as making generics work, what I wrote above doesn't have much to do with it. We could change the mapping to some macro-generated integer component Id per component class and it could work too.