Namespaces/prefixes for components' private member

[ChristophP]

When a game becomes bigger and components more numerous, I often run into the problem that components override each others properties. This should be a clear example:

Crafty.c('Lamp', {
    flash: function() {
        console.log('Flashing light intensity ' + this._strength);
    },
    _strength: 10
});

Crafty.c('Bomb', {
    explode: function() {
        console.log('Explode with intensity ' + this._strength);
    },
    _strength: 20
});

var ent = Crafty.e('Lamp, Bomb');
ent.flash(); // logs "Flashing light intensity 20"
ent.explode(); // logs "Explode with intensity 20"

I think the problem is that many components have mostly private properties, which get overridden nonetheless, when another component uses the same name. For the public properties the developer has to take care that no naming clashes occur. Sure, one could just keep track of the private names oneself as well, but this becomes very difficult as the code base grows and I think this is something that should be handled by Crafty. The best solution for how it could be fixed in the future I could come up with so far, is prefixing private members, internally with the component name(this would have to be done by the extend method) and then accessing them via a getter method(i.e. getPrivate()). So when a component Bomb is added with a _strength member, the extend method could add '_Bomb_strength' instead. That way private members would not get overridden.

Crafty.c('Bomb', {
    explode: function() {
        // accessing the private member
        console.log('Explode with intensity ' + this.getPrivate('_strength');
    },
    _strength: 20
});

How do you guys usually solve this kind of problem? I would like to hear your thoughts.

[starwed]

I definitley aggree this can be annoying -- in practice I've simply avoided declaring the same variables in different components. There's an open issue to at least warn when two components added to the same entity have declared the same property, which at least would prevent you from being surprised by unexpected conflicts.

this.getPrivate('_strength')

This technique wouldn't quite work -- when you call entity.explode(), then this will refer to the entity, with no knowledge about which component setup the explode function. So getPrivate wouldn't know what prefix to add.

I believe the standard approach with component frameworks is to keep component properties/methods completely isolated: entity.component("2D").x=5, entity.component("Bomb").explode(), and so forth. Crafty gives you the convenience of methods/properties living directly on the entity, with the inconvenience of having to manually prevent conflicts. In practice I've found this to be ok, but there are two ways we could improve it:

• Warn about conflicts as mentioned above
• Make it less likely for internal names in built-in components to conflict, probably by prefixing (i.e. the "Color" component could use _colorStrength instead of _strength)

[ChristophP]

Yeah, I see now how this.getPrivate('_strength') would not work because it would not know, what prefix to add. It is Crafty's gift and curse that members live directly on the entity. It's convenient to code but completely eliminates the availability of private members. Another very simple way of introducing private members would be by modifying the definition of the Crafty.c() function to this:

c: function (compName, component) {
    if(typeof component === 'function') {
        components[compName] = component.call(this);        
    } else {
        components[compName] = component;            
    }
},

It would then also accept a function as a second argument and could be called like this:

Crafty.c('Bomb', function () {
    var strength = 3;       
    return {
        explode: function() {
            console.log('Explode with intensity ' + strength);
        } 
    } 
});

I think this would be a nice solution, because it still compatible with the current style of creating entites, but additionally offers the possibility of using private members via closures if that is desired. What do you think of that? Sure, one could also manually prefix membernames(i.e. with the component name) but then it becomes a hassle when you decide to change the component name. Also for larger games or when using third party components its just hard to keep track of the all the names that have been used.

[ChristophP]

I realized why my suggestion above would also be problematic. When putting private members inside a closure the members would be shared across the entities. We need them to be separate for each entity though.

[theodorton]

Probably not the best solution, but you could define the member functions during the init-call. Might cause some performance issues.

Crafty.c('Lamp', {
    init: function() {
      var _strength = 10;
      this.flash = function() {
          console.log('Flashing light intensity ' + _strength);
      };
    }
});

Crafty.c('Bomb', {
    init: function() {
      var _strength = 20;
      this.explode = function() {
          console.log('Explode with intensity ' + _strength);
      };
    }
});

var ent = Crafty.e('Lamp, Bomb');
ent.flash(); // logs "Flashing light intensity 10"
ent.explode(); // logs "Explode with intensity 20"

[ChristophP]

Hey @theodorton, sorry for the late respnse, I haven't been on github in a while. Your solution would definitely work. But I do think that this would result is messy definition of the components. Just imagine if you define a large one and everything is crammed into the init function.

[matthijsgroen]

You could also have a warning mechanic on the addComponent method. when the entity is made and several components are 'merged' into a new entity, a exception could be thrown that certain keys are already taken, forcing you to change the naming.

[kevinsimper]

@matthijsgroen I tried that in https://github.com/craftyjs/Crafty/pull/713 but there were soo many exceptions to that rule that it was not worth it sadly!

[matthijsgroen]

@kevinsimper nice that you tried it already, and too bad that its not a feasible solution for this

so far I do like the solution proposed by @theodorton. But to be honest, its not a problem I have run in too so far.