In my Entity-Component-System, every entity has Com_OwnerShip component.

Com_OwnerShip = can own [0,∞) entity / can be owned by [0,1] entity

For example :-

• a rocket OWN 1 physic object  + 3 graphic object
• a rope OWN 10 physic object  + 10 graphic object
• a physic object OWN 1 Bullet(R) physic internal object
• a game chunk OWN 16*16*16 blocks

Here is a way to implement it (can be optimized further):-

class Com_OwnerShip{
    std::vector<Com_OwnerShip*> ownees;
    Com_OwnerShip* owners;
};

At the end of every time step, I list every owner that was marked as to be destroyed, then mark all corresponding ownee entities as to be destroyed.   The process is propagative.     Then, I delete all "to be destroyed" entity in a batch-style.

I have used it in a few prototypes and always find that it is very convenient.

However, I feel that it is probably a bad practice. 

• It probably share the disadvantage of Base_Game_object in OOP. (?)
• It costs a little more memory, not modular, too wildcard, too "duck-typing".
• It is a dirty-design (?).
• Nonetheless, it is better than Strong-Pointer (std::unique_ptr) because I can control entity/component deletion very easily.

Is it a good practice?     What are alternative approaches?   Am I too nervous?

Unless there's more complexity than what you're describing, I'm not sure why you dont just use smart pointers and let those deal with your object lifetime management (std::shared_ptr, std::weak_ptr).  It would be easier and less bug-prone than what you're doing now.

Also, what's the point of having a component that has to be in every single entity?  Why not just put that functionality into the entity?

38 minutes ago, 0r0d said:

Unless there's more complexity than what you're describing, I'm not sure why you dont just use smart pointers and let those deal with your object lifetime management (std::shared_ptr, std::weak_ptr).  It would be easier and less bug-prone than what you're doing now.

Also, what's the point of having a component that has to be in every single entity?  Why not just put that functionality into the entity?

Thank 0r0d.   I agree std::shared_ptr and std::weak_ptr are less error-prone.

I didn't push the functionality to the entity because I am still too scared by this idea.

There are a few cases that delete-flagging is more useful.

First Reason : safer callback

Suppose that :-

• A rocket E1 owns a physic body E2.
• The physic body E2 owns a Bullet physic internal entity E3.

In a std::unique_ptr-style : a user delete a rocket -> E1 E2 and E3 will be deleted immediately (in reverse order).  

After E3 is deleted, the Bullet physic engine will still has a dangling pointer to the physic body (btRigidBody).   It is quite hard to workaround this issue.  Collision callback (collision-end) will be a mess.

2nd Reason : more multi-threading friendly

By using flag, every entity is ensured to exist until the end of time-step.   Component iterator is easier to implement and require less mutex lock.

For me, such assumption make various parts of gameplay easier to code.

3rd Reason : cache friendly (not sure)

With flagging, deleting is executed as batch.

9 hours ago, hyyou said:

Thank 0r0d.   I agree std::shared_ptr and std::weak_ptr are less error-prone.

I didn't push the functionality to the entity because I am still too scared by this idea.

There are a few cases that delete-flagging is more useful.

First Reason : safer callback

Suppose that :-

• A rocket E1 owns a physic body E2.
• The physic body E2 owns a Bullet physic internal entity E3.

In a std::unique_ptr-style : a user delete a rocket -> E1 E2 and E3 will be deleted immediately (in reverse order).  

After E3 is deleted, the Bullet physic engine will still has a dangling pointer to the physic body (btRigidBody).   It is quite hard to workaround this issue.  Collision callback (collision-end) will be a mess.

2nd Reason : more multi-threading friendly

By using flag, every entity is ensured to exist until the end of time-step.   Component iterator is easier to implement and require less mutex lock.

For me, such assumption make various parts of gameplay easier to code.

3rd Reason : cache friendly (not sure)

With flagging, deleting is executed as batch.

 

I would be much more afraid of using raw pointers.  If you do end up with dangling smart pointers it's because your ownership was not properly defined.  I would use std::shared_ptr or std::unique_ptr (whichever is suited for your use case) for the ownership and then only store std::weak_ptr for anything that doesnt explicitly own an object/component.  Then for temporary pointers you use std::shared_ptr or raw pointers, again depending on your use cases.  If you're clear about who owns what and dont store std::shared_ptr's to things that you dont own, you should be fine.

As far as the MT requirements, that would certainly qualify as "more complexity" than what you originally talked about.  However, we're still talking about ownership.  If you use std pointers then you have to take ownership seriously.  It's something you have to put thought into, you cant just use std::shared_ptr everywhere (for example), just like you cant just use raw pointers everywhere and expect to be free from problems.   Using raw pointers only makes the situation worse. What if you forget to tag some component somewhere as "to delete"?  This is what really creates the mess, and it's why you need to tag and delete things all at the end... because that's the only way to try to clean up the mess.

Lastly, deleting in batches will probably not help you with cache performance.  Those objects/components will still be scattered all over in memory, so I doubt there will be any cache benefits.

The various types of smart pointers (and raw pointers) not only exist to make your ownership semantics clear and self-documenting, but also to make it difficult to disobey those semantics.

Raw pointers convey a lack of ownership. Using them any other way only acts to obfuscate the true ownership semantics of your code and makes it more error-prone and harder to reason about.

Case in point:

On 10/2/2017 at 1:02 AM, hyyou said:

Here is a way to implement it (can be optimized further):-

class Com_OwnerShip{
    std::vector<Com_OwnerShip*> ownees;
    Com_OwnerShip* owners;
};

There is a very serious inconsistency here. On the one hand, the single 'owners' field conveys unique ownership. On the other hand, what's stopping two entities from having a third shared entity in both of their 'ownees' vectors, i.e. shared ownership? What goes in the 'owners' field of that third entity? What happens if an entity has multiple owners, and one of those owners is destroyed? How do you handle ownership cycles?

What ownership semantics do you want? Start there and then pick the right tool for the job. Working backwards will only set yourself up for major problems later.

Thank you everyone for many useful advises.

On 10/2/2017 at 5:29 PM, Bregma said:

Short answer to your original question: if it works and lets you finish the game, it's an acceptable design.

Thank.  It is my "default" mindset. 

On 10/2/2017 at 5:42 PM, Hodgman said:

struct Rocket { Ptr<RigidBody> physics;... };

I had implemented like this.  It is very easy to understand and debug.

However, in some rare cases (10% for me), the deletion is required to be delayed.   

For Bullet physics engine, it is the best to invoke collision callback in a batch manner.  

- update game logic
--- MARK delete a rocket
- propagate the MARK to physic object
- update physic
--- collision callback 
   -> discover that the manifold between ...
       ... physic object p1-vs-p2 is no longer exist
   -> invoke Physic::collisionEnd(p1->userData,p2->userData)
- delete every thing that was MARK-ed

I believe that the correct approach is to delay to deletion of everything.  Otherwise, the p1 can be a dangling pointer.  It is also very handy to know for sure that p1->userData (which is Rocket instance) is not a dangling pointer.

Thus, I adopt the "delay deletion" practice into everything.

An alternative simpler approach is to let a system that handle rocket 1.broadcast and 2.invoke object deletion manually (like your example), but  I prefer a more automatic way.

I am not sure how game engine should support ownership relation that need the delay. I may think too much. 

14 hours ago, Zipster said:

On the other hand, what's stopping two entities from having a third shared entity in both of their 'ownees' vectors, i.e. shared ownership?   ... ... What ownership semantics do you want?

The question makes me reconsider about many things.  Thank.

I am using semantic like this.  It has run-time assertion.  I am OK about it, but not sure if it is good :-

relation_ownership->add(rocket, physicTriangle);
relation_ownership->add(rocket, graphicWing);
relation_ownership->add(rocket, barrier);
relation_ownership->add(barrier, graphicLightSphere);

I try to avoid share_pointer, because it is quite notorious - hard to debug / circle reference / slow.   I also don't need it because I know the clear ownership.

11 hours ago, hyyou said:

I had implemented like this.  It is very easy to understand and debug.

However, in some rare cases (10% for me), the deletion is required to be delayed.   

For Bullet physics engine, it is the best to invoke collision callback in a batch manner.  

- update game logic
--- MARK delete a rocket
- propagate the MARK to physic object
- update physic
--- collision callback 
   -> discover that the manifold between ...
       ... physic object p1-vs-p2 is no longer exist
   -> invoke Physic::collisionEnd(p1->userData,p2->userData)
- delete every thing that was MARK-ed

I believe that the correct approach is to delay to deletion of everything.  Otherwise, the p1 can be a dangling pointer.  It is also very handy to know for sure that p1->userData (which is Rocket instance) is not a dangling pointer.

Thus, I adopt the "delay deletion" practice into everything.

An alternative simpler approach is to let a system that handle rocket 1.broadcast and 2.invoke object deletion manually (like your example), but  I prefer a more automatic way.

I am not sure how game engine should support ownership relation that need the delay. I may think too much. 

You could also have the physics engine own the user data, and allow the game objects to access it. That way the object could be deleted immediately and the 'delete me' flag could be in the user data. I'm sure there's also a way to allow the physics engine to clean up the object immediately, but I'm not familiar with Bullet so I can't comment on any specific limitations or best practices.

But the point is that there's often a way around deferred deletion and other tricks... if you rethink the data ownership a bit

11 hours ago, hyyou said:

I am using semantic like this.  It has run-time assertion.  I am OK about it, but not sure if it is good :-

relation_ownership->add(rocket, physicTriangle);
relation_ownership->add(rocket, graphicWing);
relation_ownership->add(rocket, barrier);
relation_ownership->add(barrier, graphicLightSphere);

I try to avoid share_pointer, because it is quite notorious - hard to debug / circle reference / slow.   I also don't need it because I know the clear ownership.

From what you've described, unique ownership should be sufficient. Shared ownership isn't required as often as one might think.