benmoran56 / esper

An ECS (Entity Component System) for Python
MIT License
544 stars 69 forks source link

component remove callback #24

Closed DeniZka closed 6 years ago

DeniZka commented 7 years ago

I used esper with pyglet and pymunk. So i create vertex_list for graphics component and bodies for physics components. When i removed those components i used del method to release objects. But cause of python, sometimes it crash an app. So i made child class of esper.World which call a components callback on delete_entity method. There i clean component before it leave the World

benmoran56 commented 7 years ago

Hi DeniZka,

Are you using the vertex_list.delete() method, called by __del__ of the component class? In my experience, that should work.

Can you show me an example of your child class? Maybe we can improve esper itself so this is not necessary.

DeniZka commented 7 years ago

Hello! Ok. I pushed my project into github. it uses esper, pyglet and pymunk. You can try initial commit 8dfbecc when project was at the start steps. (Just kill the blue square. arrows=move Shot=Return button) everything ok. The problems appears when the project became more complicated. In the last commit:

benmoran56 commented 7 years ago

I had a try, and I couldn't get it to crash. What kind of traceback are you seeing?

Also, you are using esper in a bit of a strange way. Following ECS designs, the Components should not have and any methods (besides init). The methods that you have on your Components, should be in the Processors instead. It might be easier to prevent conflicts in this way.

I'm sorry that I don't have a better example with esper, but you can see a (buggy) game example here: https://bitbucket.org/HigashiNoKaze/ripple-pyweek22/src/ab4e702b05d0864bd0ee50ce848c019660850cde?at=master

DeniZka commented 7 years ago

I saw this only in debug mode: 'Chipmunk error: This operation cannot be done safely during a call to cpSpaceStep() or during a query. Put these calls into a post-step callback. Failed condition: !space->locked' entity removed but Graphics hangs and physics continue work. Problem in python destructor call time. Ok. I understand about ECS rules. If it so, there is no need in "improvement" :). Thank for advice.

benmoran56 commented 7 years ago

Oh, interesting. That makes sense.

Is the pymunk step command being called from inside a processor? It might help if this processor has a very hight "priority" setting, so that it's called last (or first). I'm not sure as I havn't used pymunk in a long time. Refactoring your project would be a good start, though, and help to troubleshoot easier.

DeniZka commented 7 years ago

Physics processor has right order, another places can make glitches. Another thing i just implemented: I think it must be callbacks on processors added (removed) to world. It could help with processors initial (final) interconnections or subscriptions for some actions.

DeniZka commented 7 years ago

I found a problem! I have more then one ref for entity so python does not call destructor!

benmoran56 commented 7 years ago

That's great! I'm glad you found it.

About Processors, do you say that you Dynamically add/remove processors to your code, during execution?

DeniZka commented 7 years ago

Yes and no. Yes. For example i have additional processor for Edit mode. On adding to the world it subscribes for some of actions on_mouse_... on_key_... in other processor in the world. On remove it unsubscrubes. No. For example i have some static of processors uses Camera processor to interact with. So there are a lot of copy of code to check Camera processor is available now in the world. Or i just enable all functionality when Camera added. Of cource i could just make an right order for world initializing, but.. i think it more comfortable.

I created a World child with this code:

    def add_processor(self, processor_instance, priority=0):
        #every one must know about new one
        for prc in self._processors:
            prc.on_add(processor_instance)
        #add new one
        super().add_processor(processor_instance, priority)
        #tell new one that it in game
        processor_instance.on_add(processor_instance)

And for Processor child:

    def on_add(self, processor):
        pass

    def on_remove(self, processor):
        pass
benmoran56 commented 6 years ago

Sorry for the delay. That's an interesting use of the Processors. I'm not sure if this functionality should be added to esper, because it's outside of the typical ECS pattern. I don't mean to say it's wrong, but I'd like to have esper folow "pure ecs" concepts as much as possible.

DeniZka commented 6 years ago

That's ok. We got fork button in github. Thank you

benmoran56 commented 6 years ago

Hi @DeniZka, No worries. I'm happy if you're able to find the library useful. Please do go ahead and fork it.

I'd also say, and please don't take this the wrong way, but you might want to try refactoring it with a more "pure" ECS design at some point in the future. There is nothing wrong with your aproach, and already pretty far along already. However, a more pure ECS layout would be a bit easier to refactor I think. I really need to work on some better documentation and code examples on my end as well. Please feel free to ignore that advice, and I'll be checking on your project from time to time!

-Ben