Search Unity

SystemStateComponentData and Prefab

Discussion in 'Entity Component System' started by Flipps, Jan 18, 2020.

  1. Flipps

    Flipps

    Joined:
    Jul 30, 2019
    Posts:
    51
    If i instantiate an Entity from an Entity Prefab with a SystemStateComponentData on it, the SystemStateComponentData is not copied (like all other components) to the new instantiated entity.

    Prefab Entity with TestSystemStateComponent and normal TestComponent attached:
    upload_2020-1-18_18-10-21.png

    Instantiated Entity where only TestComponent is copied:
    upload_2020-1-18_18-10-38.png

    Is this working as intended? I think it would be really nice to setup SystemStateComponents in Prefabs, too. Otherwise there is always a system required to add SystemStateComponents at creation.
     

    Attached Files:

    Last edited: Jan 18, 2020
  2. MaNaRz

    MaNaRz

    Joined:
    Aug 24, 2017
    Posts:
    117
    I got completly blindsided by this behaviour today. This behaviour should at least be documented somewhere. Instantiating an Entity never copies ISystemStateComponents. It does't matter if it is from a Prefab or from another instance of an Entity. As was pointed out above this leads to having to create additional setup logic that could be run in Conversion.
    In my usecase whenever an Ability spawns i need to setup those ISystemStateComponents which makes the abilities change chunk atleast once.
    It is actually even worse for me because i instantiate an Ability hierarchy which could have multiple Entities that need this setup logic.
    I guess i will think about other ways to clean up behind Abilities instead of ISystemStateComponent (altough that would be their intended use based on the documentation)...

    Can someone explain why this decision was made?
     
    Last edited: May 4, 2022
  3. Enzi

    Enzi

    Joined:
    Jan 28, 2013
    Posts:
    962
    I think it's just an oversight that needs attention.
    A reasonable logic would be that ISystemStateComponents are never copied, unless they are on a Prefab.
    That way one could set it up in both ways.
     
  4. MaNaRz

    MaNaRz

    Joined:
    Aug 24, 2017
    Posts:
    117
    I would actually prefer if the diffrence in instantiating ICompData and ISystemStateCompData would be removed entirely. I'd say usually if you want to Instanciate a Copy of an EntityInstance you would also want to copy how the entity is set up. If you spawn a Prefab you had full control over whether or not to add SystemState in the Conversion Step you wouldn't add it by mistake. So unless there is some internal usecase Unity needs I just don't see the advantage of handling it diffrently.
     
  5. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,264
    Can someone please tell me what kind of component can be instantiated and copied blindly but requires a system to clean up? Either I am missing something or you all are trying to use SystemState components wrong, because allowing the data to be implicitly copied around on instantiation, prefab or not, would break a lot of things.
     
    iamarugin likes this.
  6. MaNaRz

    MaNaRz

    Joined:
    Aug 24, 2017
    Posts:
    117
    I realize now I didn't explain my usecase well enough:
    So I have an EffectEntity that can be instantiated as a Child of an AbilityEntity. The Effect has a Buffer to keep track of Entities it has effected. This Buffer is filled whenever the Ability hits a Target. When the Effect is destroyed i want to reverse the Effect on all Targets. Handling Effects this way basically allows me to only use reactive Systems and not have any per frame costs for them (All Effects only share a single System that counts down their Lifetime).
    This Buffer of EffectedEntities will always be empty on Instantiation of the Ability but is needed for clean up. That means when im using SystemStateComponents I always have to add that Buffer in the first frame of existance and need extra Setup Systems at the right point in the Frame to not get unwanted SyncPoints due to structural changes. In my case the cleanup logic can be performed in different ways based on the settings of the Effect (defined in the GameObjectPrefab). So i need this SettingsComponent as an ISystemStateComponent too. That means when im instanciating a Prefab i always have to manually copy over the Settings into an ISystemState and add the empty ISystemStateBuffer. Both things could be done in Conversion without any Problems.
    Now to make this even more complicated I use "ProtoTypeAbilities" per Unit to enable per Unit Buffs affecting those PrototypeAbilities. When Units use an Ability I Instantiate this modified Prototype instead of the Prefab. So now I need to copy and manually add this ISystemState I could have setup right at the Conversion Step whenever a Unit spawns or a Unit uses an Ability.
    This overhead would all go away if SystemState would be copied when Instantiatiaing.
    I am not saying I need this to make anything work.

    I am arguing that i don't see the merits of not doing it like this. It's just an extra rule I have to keep in mind / learn. And if it needs to have the behaviour it has now it must be documented somewhere.


    In the Docs it says the intended use is for clean up. I think my use would fall into that category. I am curious as to what would break right now. You looked much deeper into the source than I ever could. I am having trouble imagining a case.
     
  7. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,264
    Let me provide a little example of where things might fall apart in your example.

    So first, let's imagine some time in the future you write a new mechanic where by some other criteria, your effect projectile can replicate with each replicated instance traveling in a slightly different direction. Now let's assume the effect projectile has already affected an entity or two before this happens. Then when it happens, you decide to implement it by instantiating the live effect. Now you have multiple copies of the buffer and consequently when all those new projectiles get destroyed, the originally affected entities before the split will have their effect reversed multiple times.

    The reason it is called SystemState is because a system can have complete ownership over the values and lifecycle of the data. It is owned by the system, and allows the system to make assumptions that things won't be changed from underneath.

    Now you can almost make an argument that having a default state in a prefab would be fine. After all, that state could include a bool that states whether or not the data has been initialized. The problem with that is that it is common to instantiate a prefab, modify some values and add some components based on runtime data, and then instantiate that many more times. The rule is better kept simple.
     
    iamarugin likes this.
  8. MaNaRz

    MaNaRz

    Joined:
    Aug 24, 2017
    Posts:
    117
    Valid point where it could break but it's not like i couln't change the SystemState when i spawn the Entities just like I would adjust Translation to a Spawnpoint with IComponentData.
    And in my example that's what I use Prototypes for. I already got a "Split" Ability that would work just fine with it. I would have control over it instead of not having a choice in the matter and requiring structural changes.

    Again I agree that it is a nice pattern to have a Component only controlled by a single System. But this is not enforced anywhere at all. At runtime i can break that pattern as much as i want to because nothing enforces that "rule" of ownership by the System. But why then can't i have a ConversionSystem setting up the basic Archetype to not have to do structural changes every single time an Entity is Instanciated. Nobody would do that as a mistake. That would be a concious choice.
    How is Unity doing it with the Child ISystemStateBuffer btw.? Are they rebuilding it from scratch when a Prefab is spawned or are they somehow just remapping it? This could be one point where it would currently break if a not remapped Child Buffer was copied.


    Yes but on the other hand the rule would also be simpler if there weren't extra rules of Instantiation for ISystemState. Everyone is aware that some ComponentData needs to be set when Instantiating an Entity.
     
  9. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,264
    For your code that may be true. But if Unity or someone else were to ship an animation package that requires SystemState components across the entire hierarchy, would you want to touch that? Do you want to have to clear out the Child buffer on every entity at every depth level every time you instantiate one (since things parented at runtime wouldn't get remapped)?

    Yes

    The rule for SystemState is as simple as it gets. It is a component type that is ignored by instantiate and destroy operations and must be explicitly added and removed. If you don't want that behavior, don't use SystemState.

    It is less to prevent people from doing that and more because there is no way to differentiate doing that and making an actual mistake. Plus requiring anything that initializes a SystemState to have to account for either a missing component or an uninitialized component would complicate a lot of those systems, not to mention the added state they would have to store in the chunk to detect that a component is unitialized, as well as the cost of polling for uninitialized components.
     
    MaNaRz and Luxxuor like this.
  10. MaNaRz

    MaNaRz

    Joined:
    Aug 24, 2017
    Posts:
    117
    Thank you! The reasons of ISystemState instantiation behaviour now make more sense to me. I wouldn't want UT to change anything if that has a lot of consequences. It's just that the consequences weren't clear to me.

    After this brief discussion this earlier statement:
    now becomes:
    It needs to be documented somewhere.

    The current documentation about ISystemState only talks about its different destruction behaviour.
     
    Last edited: May 6, 2022
  11. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,264
    It is mentioned in the documentation for EntityManager.Instantiate. But besides that, I agree the documentation is a complete miss regarding this.