Game engine utilizing a custom graphics system

Question

I just have a somewhat simple question on coding practices. I have never written a large application before and I am currently working on building a game engine in JavaScript. The part that confuses me is what the best method of organization is in this particular case.

I have my base engine class, Engine, a graphics class, GFXSystem, and I eventually plan on adding a physics system class. I am utilizing the entity component model, so each object in the game world is represented as an entity and various data classes, called components, are added to an entity to represent its state and allow it to do things.

Some components need to report to the GFXSystem so that it knows to include them when it renders to the screen. The engine class is the all encompassing class that keeps track of both the entities and the graphics system, and calls the update method each frame to run update on each component on each entity and tells the GFXSystem to draw the next frame. An engine will always have one GFXSystem.

I started off exposing both the engine and the graphics system to window. I decided against this and then tried letting the engine have a reference to the graphics system and instead had the entity store a reference to the engine it was attached to. This then resulted in the components having to call the following to tell the GFX system that it needed to be rendered, which seems wrong:

this.entity.engine.gfx.addToDraw(this)

Is that really how I should be doing things or is there some better structure I could follow? Not all components even need to do anything outside their own data.

Here is a sample of some of my code, bear with me, it is a bit hastily done since I am still in the initial stage of writing things just to get it to work.

var Engine = function(){
    this.graphics = new GFXSystem();
    this.keyboard;
    this.keys;
    this.mouse;
    this.__entityList = new EntityList();
    //window.engine = this;
    //window.renderer = this.__renderer;

    this.start = function(){
        this.graphics.start();
        this.keyboard = new Keyboard();
        this.keys = new Keys();
        this.mouse = new Mouse();
    }

    this.update = function(time){
        for(var handle = this.__entityList.head; handle != null; handle = handle.next){
            handle.entity.updateComponents(time);
        }
        this.graphics.update(time);
    };

    this.addEntity = function(entity){
        this.__entityList.Add(entity);
        entity.engine = this;
    };
};

Graphics

var GFXSystem = function(){
    this.renderer = null;
    this.width = document.body.clientWidth;
    this.height = document.body.clientHeight;
    this.camera = null;
    this.scene = null;
    this.__list = new EntityList();

    this.start = function(){
        this.renderer = new THREE.WebGLRenderer({antialias: true});
        this.renderer.setSize(this.width, this.height);
        document.body.appendChild(this.renderer.domElement);
        this.renderer.setClearColorHex(0xeeeeee, 1.0);
        this.renderer.clear();

        //this.camera = new THREE.PerspectiveCamera(45, this.width/this.height, 1, 10000);
        //this.camera.position.z = 300;

        this.scene = new THREE.Scene();
    }

    this.update = function(time){
        /*
        this.camera.position.x = Math.sin(time/1000)*300;
        this.camera.position.y = 150;
        this.camera.position.z = Math.cos(time/1000)*300;
        */
        // you need to update lookAt on every frame


        this.renderer.render(this.scene, this.camera.camera);
    }

    this.registerEntity = function(entity){
        this.__list.Add(entity);
        this.scene.add(entity.getComponent('render').model);
    }

    this.removeEntity = function(entity){
        this.__list.Remove(entity);
        this.scene.remove(entity.getComponent('render').model);
    }
}

Component

var RenderComponent = function(model){
    this.model = model;

    this.start = function(){
        var pos = this.entity.getComponent('position');
        model.position.x = pos.x;
        model.position.y = pos.y;
        model.position.z = pos.z;
        this.entity.engine.graphics.renderer.registerEntity(this.entity);
    };

    this.update = function(time){
        var pos = this.entity.getComponent('position');
        model.position.x = pos.x;
        model.position.y = pos.y;
        model.position.z = pos.z;
    };

    this.stop = function(){
        this.entity.engine.graphics.removeEntity(this.entity);
    };
};

RenderComponent.prototype = new Component();

My old code set window.graphics = this.graphics and window.engine = this.engine inside of the engine's start method.

Answer 1

First of all, it's a bit surprising that you roll your own EntityList. I would expect an normal array to be more efficient and natural, at least when starting.

A good mindset when thinking about coupling between parts of your code is to think about the specific problem of testing your code for correctness. It's a very real problem than you can solve, while "having code that looks properly designed" is not useful in itself.

The first thing is that you should not expose global variables to window as it will make your code impossible to test: those variables will taint everything interesting in your code. The current code is still very much tied to the way the entity stores the engine, and the way your engine stores graphics. I've heard a lot of praise about the entity component model, but unfortunately I don't know how it's tested.

A related issue to think about first is the way you model requirements between different parts of your code: take a look at the module pattern and consider using something like RequireJS to make dependency handling easier to manage.

I guess there's no good way to separate the rendering part of the component from the graphics system, so it's OK to have coupling here, but avoid relying on knowing graphics/engine when you don't have to.

Answer 2

You should be modifying prototype, instead of assigning this.method inside your constructor. Your constructor runs every time you instantiate your class; you're effectively rerunning the entire class definition, every time. Each instance of your class will have it's own set of functions, completely with all the associated overhead.

This:

var Engine = function(){
  // ... 
  this.start = function(){
    // ...
  }

  this.update = function(time){
    // ...
  };
}

Should be this:

var Engine = function () {

};

Engine.prototype.start = function () {

}

Engine.prototype.update = function () {

}