Skip to content

Conversation

@SleepProgger
Copy link
Contributor

There are multiple problems regarding components added by create_component / removed by remove_component.

Components created via create_component aren't removed at the gameworld.remove_entity call.

        gameworld.clear_entities()
        system = gameworld.system_manager['position']
        ent = gameworld.init_entity({'rotate':0}, ['rotate'])
        system.create_component(ent, 'general', (0,0))
        self.gameworld.remove_entity(ent)
        assert system.get_active_component_count() == 0

Initial components removed by remove_component lead to a crash at gameworld.remove_entity.

        gameworld.clear_entities()
        system = gameworld.system_manager['position']
        ent = gameworld.init_entity({'rotate':0, 'position':(0,0)}, ['rotate','position'])
        cind = gameworld.entities[ent].get_component_index('position')
        system.remove_component(cind)
        self.gameworld.remove_entity(ent)

Duplicate components lead to memory leaks as the second component will overwrite the first one without any warning whatsoever.

    gameworld.clear_entities()
    system = gameworld.system_manager['position']
    ent = gameworld.init_entity({'rotate':0, 'position':(0,0)}, ['rotate','position'])
    system.create_component(ent, 'general', (0,0))
    self.gameworld.remove_entity(ent)
    assert system.get_active_component_count() == 0

This PR fixes/prevents all the mentioned cases.

**Changes:**
- Changed the `EntityManager.set_component` method to call `Entity.set_component` so the `load_order` can be updated. Not super happy about this but i think its the best backwards compatible way to do this.
- The `Entity.set_component` function checks if the system is already in the `load_order` and otherwise adds it. This makes component initialization a little bit slower because we need to search in the `load_order` list. This could be optimized, but according to my benchmarks it doesn't really matter.
- The `Entity.set_component` function will now raise a `DuplicateComponentError` if there is already a component from the system active.
- The Entities `load_order` now saves the system indices instead of the names as all the places using them need the system index anyway.
- The Entities `load_order` property still returns the system_names but i removed the setter as changing them directly is just asking for trouble IMHO.


@SleepProgger
Copy link
Contributor Author

Unrelated, but can we get the travis build fixed please ? Seem to be a problem with opengl libs ?

@Kovak
Copy link
Contributor

Kovak commented Sep 14, 2017

👍 awesome work here, this is an area that really needed to be fleshed out. I'll look into fixing the travis build. It seems to break reaaally frequently and then fix itself every now and then. Don't know that much about travis.

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

Successfully merging this pull request may close these issues.

2 participants