Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Feature request: Deleted entities discard ecb writes

Discussion in 'Entity Component System' started by cultureulterior, Oct 2, 2020.

  1. cultureulterior

    cultureulterior

    Joined:
    Mar 15, 2015
    Posts:
    68
    It would be really nice if we could get a flag in EntityCommandBuffers AddComponent and AddBuffer to discard writes to deleted entities- there's no real way to be sure that an entity survives to the end of the time of the system without being very careful about scheduling.
     
    Lieene-Guo likes this.
  2. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
  3. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    Also, ECB can tell exactly if the entity is destroyed or not valid.
    Each world has an entity id range, negative entity means ecb cached. version can tell if the entity was used or not.
    So ECB knows if the entity was created and used then destroyed, for those entities ECB should just ignore add/remove/etc. Instead of throw exception and give up the rest of the commands.
     
    Last edited: Oct 3, 2020
  4. unity-freestyle

    unity-freestyle

    Joined:
    Aug 26, 2015
    Posts:
    45
    While I get the point, isn't it an architectural problem though?

    I think the problem can be structured in ways where enqueueing commands for destroyed entities (which are going to be destroyed before the playback) can be avoided.

    Assuming that all entities that are going to be processed in the command buffer exists and are valid also has its advantages.
     
    Last edited: Oct 4, 2020
  5. cultureulterior

    cultureulterior

    Joined:
    Mar 15, 2015
    Posts:
    68
    Sure, you can work around it, by carefully tracking everything, but this seems like an extremely easy thing to add, and would just improve the DOTS programmer experience.
     
  6. unity-freestyle

    unity-freestyle

    Joined:
    Aug 26, 2015
    Posts:
    45
    Well, I disagree.

    I think it's a matter of structuring entity life cycle, which will not require "carefully tracking everything". It could lead to a debugging experience instead.

    But if added as overloads, with warnings and without perf impacts, I actually have no objections.
     
  7. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    It's a matter of what should be considered an Exception.
    if you look into my post:
    https://forum.unity.com/threads/arg...es-not-exist-from-entitycommandbuffer.979899/
    Adding a component that is already added to the target entity is skipped without any log info.
    But Adding a component to an entity that was used and destroyed is an Exception.
    IMO, it's just a bug.
    For Detial:
    within AddComponentWithValidation the safety check AssertCanAddComponent need the Entity to exist and [Conditional("ENABLE_UNITY_COLLECTIONS_CHECKS")] can not have return value. so it through Exception and give up all the rest of the commands. That is the worst case I could expect!
    But, AddComponentWithValidation can skip the case that the entity is destroied. It is just the check who needs the entity to exist...

    An extra EntityWasExist test and return false will fix this problem...
    If a warning is expected, both HasComponent and EntityWasExist need to log warning.
    It looks like what is Exception and what is not is decided in a way that I cannot understand.
     
    Last edited: Oct 5, 2020
  8. tertle

    tertle

    Joined:
    Jan 25, 2011
    Posts:
    3,753
    This was not always the case and I disagreed when they changed it.
    It has hidden quite a few errors over the last couple of years.

    Anyway this is not the most user friendly approach so it wouldn't shock me if the behaviour changed at some point but I'm a bit of a purist though so personally I agree with unity-freestyle. I feel like if you are running into this error it's more an issue of not well designed architecture that could probably be improved.
     
    PublicEnumE likes this.
  9. optimise

    optimise

    Joined:
    Jan 22, 2014
    Posts:
    2,113
    There's a feature called Batched Playback they are look into at 2019 but until now it's still not happen yet. Basically Batched Playback will instantiate entity at batch then add/set/remove component at batch then destroy entity at batch. I believe when this feature lands it will kind of solve this problem.
     
    PublicEnumE likes this.
  10. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    I understand and agree that DOTS should use a strick error stop pattern.
    But for deferred commands like ECB. It is another story. ECS cannot force every system to use the same ECB(not ECBS but ECB) or ECB will lose its design goal.
    But entity destroy is not tracked cross ECB so deferred entity destruction can not be tracked anyhow.
    For now, I am using an extra ECBS after EndSimECBS dedicated to entity destruction. So any add/change/remove component happens in EndSimECBS before any entity gets destroyed.
    It's the only way I can think of, other than force every system to use the same ECB.
     
  11. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    That is exactly what EntityCommandBuffer does.
     
  12. WAYNGames

    WAYNGames

    Joined:
    Mar 16, 2019
    Posts:
    988
    Never ran in the issue but I would probably make an event system where I signal wich entities to destroy.
    One or more system(s) then consume the event just before the ECB system and clean up entitiy references on other component if any and queue the destroy command on the ECB.
     
  13. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    First, I have made my general walk-around of this issue. It's annoying but it could be done.
    I just don't want my team who are working on our game project to run into problems that out of there scope of work. It could be several guys working on the same project. One working on target destroys the other works on powerUp/Buff. And they would have to align their code on none-game logic details for issues like deferred entity destruction.ESC is perfect in terms of de-coupling logic, but ECB being not able to Identify Entity destroy will ruin all the effort of de-coupling.

    And, I have several event systems, trust me it makes it worse.
    Depending on event system's order. later event system's event could be postponed to the next frame. When the entity is already destroyed.
    Then you need to check all recorded entity's existence before processing event. That's unnecessary and slow as entity exist can only be tested by EntityManager.
    For example:
    EventSystem B runs after EventSystem A and SystemBase C runs after B(consuming event in B) and sends Event to A. Then the event must be postponed to the next frame. If it happens to be an add component command, and the Entity is destroyed. Then you are doomed, Event System A will have to check all entity's existence.
     
    Last edited: Oct 5, 2020
  14. WAYNGames

    WAYNGames

    Joined:
    Mar 16, 2019
    Posts:
    988
    I get your exemple but there is nothing to do if the entity is destroyed and your event is already in the queue for the next frame.
    Either you make sure your events don't cross a frame or you need to skip the event if it relies on an entity that is destroyed by then.

    In your post you said you used another ECB after the EndOfSim ECB. That's the same thing has the event system I mentioned and make no difference in the cross frame event case you mention.

    Testing the entity existance is expensive you say, so why not keep an array of the deleted entities in the previous frame (populated by the destroy entity event system) so that you can use that to check if the entity was destroyed before consuming the event ?

    Even with multiple people working on different system in the same project, if you have a convention about testing entity existence and destruction, you'll be ok.

    Testing that your game logic don't crash is part of the game logic IMO. When you need to rely on entity reference you have to manage the consequences.

    If a member of your team make a system that relies on an entity refrence (for exemple in an IComponentData), I believe he should also make the system that take care of cleaning up that reference. Keeping track of the "to be destroyed"/"destroyed last frame" entities allows you to make such clean up system. The additionnal ECB don't allow you to make that.
     
  15. Lieene-Guo

    Lieene-Guo

    Joined:
    Aug 20, 2013
    Posts:
    547
    I understand your point and most people who do not agree with looser ECB exception check have the same point of view as you do. that was reasonable.
    But a system like DOTS/ECS has thrown the problem to game system engineer like us. And I just don't want to throw the problem out again. I am looking for a general solution that my team who makes game based on my system does not need to worry too much about technical details. I want to solve it in my system! But I found that the only best solution is to change ECB, you know how it feels?

    Maybe, just maybe a New API in ECB and ECB parallel: Add/Remove/SetComponentWhenEntityExist()
     
  16. WAYNGames

    WAYNGames

    Joined:
    Mar 16, 2019
    Posts:
    988
    Honestly I don't use the ECB to set data, I usually have the data to write to in my entities foreach or jobchunk. I use it only for structural change add/remove/create/destroy.
    For my ability system I pre-process my event to map them to an entity/event map. Then I iterate over all entities that have the data needed to process the event and skip the entities that are not in the map. If one of my target was destroyed, I would not have it in the entities foreach so I just don't process the associated events. No need to wor on things that don't exist.

    EDIT : Add/Remove would indeed benefit from not crashing when the entity was destroyed by a previous command in the ECB. But you could also use the end of sim for add/remove and the begin simulation ECB for destroy.
     
    Last edited: Oct 5, 2020
    unity-freestyle likes this.