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

Feedback ECS Chronos Foundation

Discussion in 'Entity Component System' started by JesOb, Apr 10, 2020.

  1. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    The turret is just an example - I don't think that workarounds for that one situation really address the more general issue that I'm raising.

    My point is about any situation where you're reading a value from another entity, and that value is in a unit that involves time. Changing that entity's local timescale changes the meaning of the units, but there's no good way to account for that when reading the value. In the case of the turret example this could be approximated another way by having the turret measure the change in position, but that's not the case for every value with a time based unit.

    Even if workarounds always exist, people aren't going to want to always do things the hard way to account for the timescale feature. The method you're proposing not only requires more effort to build - it's worse for performance too. Most systems and assets that use these features won't end up supporting timescales properly.

    The problem scenario is relatively rare, but I don't think that it's so rare that it's okay to break it.

    Does anyone in this thread have any idea how practical it would be to provide a very fast
    GetTimescale(Entity entity)
    method? It would need to abstract away the problem of checking for the timescale being set in multiple places, and be fast enough that people aren't discouraged from using it when appropriate.

    @Joachim_Ante said "The actual important part here is to make an API that is simple enough that everyone uses it correctly without fail." I don't think the current proposal meets that requirement with respect to reading values from other entities.
     
  2. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    I agree with in case that many systems will not be compatible with timescale.
    I dont think it is possible to create silver bullet system at all :)

    GameObject.Transform not compatible with deep hierarchies, packages from store not compatible with project folder structure and many of them dont allow to move from Project Root...

    What I want to say is that (IMHO) idea where there is no compatibility with timescale at all and everything in store guaranteed not compatible with timescale and you need to hack absolutely every asset that you buy to make it compatible with timescale is more bad than idea where we have simple timescale support in core (standardized by Unity) and some asset will support it directly another ones support it because they simple and just work and some part of assets will behave incorrect when timescaled (only this part we need to hack or ask author to support standardized timescale).

    Usability of timescale not only in gameplay but in menu too.
    Today to stop entire world (with 3rd pary asset) but play 3d menu in normal time (with 3rd pary asset) you just need to hack all that 3rd pary assets. e.g. every tween lib will just work and we can use them in gameplay and menu with ease.

    Without standard we have no choice. Just hack asset for our needs. No one will support Timescale and we can not ask author to support timescale just because it is not standard thing for unity.
     
  3. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    I do understand where you're coming from. It's hard to conceive of a better system than the one you're proposing, and that brings us to the question of which form of brokenness is preferable - a messy ecosystem, or not having the ability to do it at all. I think there are good arguments to be made in both directions there.

    One thing I don't fully understand is why this needs to be a core engine feature. You say that it's because you want integration with the standard Unity systems, but it would be easy enough for them to all provide their own timescale components, and that would actually give a greater amount of control. Your points about third party assets are just as easily addressed by them adding custom timescale components as they are by the automatic system.

    The point of integrating it into the engine seems to be that users' systems automatically work with it without any extra effort, but as we've agreed that's not always going to be the case. I believe that it's worse to have it sometimes be magic and sometimes not, than it is to just always make people do it manually. That's confusing, and a surefire way to get lots of bugs and misunderstandings.

    I also don't think that it actually improves on the situation with regards to automatic support in third party assets and having to hack them. If the support is automatic, that means that most asset authors won't have carefully designed around it, and won't have clear documentation about the level of support. The only way we'll know whether an asset supports it is by experimentation and manual examination of the source. That's not actually much better than having to hack it ourselves. Any asset that does take the time to design around it, test it, and document it is an asset that could have just added their own custom timescales anyway.

    There's also still the issue that any system that wants to support it in the communication between entities scenario incurs both a design and performance cost, even in the case where the entities involved don't have modified timescales. That goes against one of @Joachim_Ante's other edicts - that this shouldn't incur a penalty when not being used. This strongly discourages authors from supporting it unless they have a very specific reason to do so, making it less likely to work automatically by default.

    If you were writing a best practice guide for asset authors, what would you advise them to do? Would you say that they should take on the burden of supporting timescales because somebody may want to use their asset in a game that uses timescales? Or should they not unless specifically requested, because it's easier and faster? I believe that if there isn't a clear answer to what the best practice is, that's an indication of a problem and will lead to a messy fragmentation of the ecosystem.

    I would love to find a silver bullet design that takes care of all of this, but I agree with you that that's unlikely. The engine doesn't know which values would need to be scaled in which ways in which places, so there's no way to do it all automatically. Solutions like annotating fields with the time perspective they should be interpreted in seem Too Magic and destined to fail. Fast and convenient access to timescales of other entities would help devs fall into the pit of success, but I don't think that's necessarily sufficient.

    I really don't mean to keep being negative about this as I would really like to have this capability in the engine, but I do think that the design proposal at least needs to directly tackle this issue in some way. The inclusion of a
    GetTimescale(Entity entity)
    method seems like a bare minimum requirement.
     
  4. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    Solution proposed/accepted by Aras is nearly zero performance and will be supported by Unity so we end up with core system that has perfomance by default and may be even better because Unity in optimization pass can make it even faster inside core, may special bursted etc. All this just impossible with custom implementation from each asset author.

    Now (in GameObject world) we dont have any core or standard from Unity about timescale and we end up with asset store with asset that dont support any timescale at all. The same will be in DOTS if Unity dont provide default core way to deal with TimeScale. When Unity dont have any TimeScale asset authors even dont think about supporting something like this. If Unity DOTS will have core timescale solution than authors start thinking about supporting it directly and even this is big step forward for ecosystem (IMHO)

    Additional note is developing with say 10 assets from store with custom timescale system foreach can lead to having to add 10 timescale components on single entity and additional code to sync all this every here and there.

    I think there we (users that want to use timescale) need to think about asset we use and the way it works every time we want to use it in timescale scenario. So dont think that everything will just work no matter what :) This we can actually say about any asset in store nowadays. Because in most cases then dont just work with out project setup and we have to deal with various integration bugs, so nothing new :)

    Somewhat Agree :) But event here I think having core system will be better because we will have one timescale component, not one per asset. I suppose that assets that directly support TimeScale will directly point this like we have today for URP, HDRP

    About 'have carefully designed' you right but without it we will have nothing. As you understand I vote more for Partial Support less for lack of support.

    And again it all about assets that do complicated things, not about simple one.
    We actually have very few complicated assets from store. I expect 80/20 so 80% of assets will just work with timescale and 20% will not or not entirely.

    Just dont agree with you there. It is true for Per Entity timescale and not for SCD timescale. It is what Aras said :)

    I will suggest them to think a little bit about timescale support but dont implement if they dont want to.
    And if they support timescale directly or by default (because timescale independant) than just add [Timescale support] tag to asset in store.

    I think I understand your fears but again think that having 80/20 support is better then nothing. And when we will have first basic support we can grow with it.

    GetTimescale(Entity) looks like random access and bad performance by design so I Personally dont want to have method like this in DOTS core. It looks like Camera.main that is bad design and we dont want to use but can not remove from Unity because it will break existing code.
     
    SamOld likes this.
  5. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    I understand where you're coming from. I think that I still disagree on the messy 80/20 thing, but your position is as legitimate as mine and I don't particularly have any new points to make. It's up to Unity to look at the pros and cons and choose what they want to do now. You argue your position very well - I think we just have different priorities in what we consider to be good design.

    I will pick up on just a few individual points.

    This is achievable in other ways, like a prominent best practices guide for asset authors in the docs.

    I think that we have a misunderstanding here. I specifically mean in the scenario where you're reading a value in time-sensitive units from another entity with
    SystemBase.GetComponent<T>(Entity)
    - like my turret example. If that other entity is provided in something like a Target component, then it is random access and you know nothing about it, including its timescale. If you need to know its timescale, you need to do a random access lookup for it. That applies even in the case where its timescale is unaltered, because you can't know that. That means that in order to support timescales, you need to add that extra random access lookup every time you process the entity - just in case that entity is using timescales.

    The way to do that, and to abstract over the fact that the timescale value could come from different places - global, per world, per chunk, probably not per entity - is with a
    SystemBase.GetTimescale(Entity)
    method. I agree with what you say about it being bad performance by design - that was the point I was making. This feature necessitates it, and that's a problem.

    It's not analogous to
    Camera.main
    , it's analogous to
    SystemBase.GetComponent<T>(Entity)
    .

    One way to mitigate the costs of that would be to keep a flag on the
    World
    that indicates whether any entity is using a modified timescale. If none are,
    GetTimescale
    could return immediately without doing the lookup. That helps, but the the issue comes back as soon as one entity uses a timescale.
     
  6. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    Oops, I forgot one thing.

    That would be a little bit fiddly, but isn't it better because it allows people to change the timescales of different systems independently? I can imagine, for example, that I might want physics to run slow-mo while animation runs full speed. Or maybe I want everything to run slow apart from the audio source. Maybe I want to create a slow-mo effect by slowing the animation, but I don't actually need to slow any other system. By making it global, you take away that level of control.
     
  7. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    I think this case not a good reason to bloat ecosystem with many timescale systems :)
    It is achievable with few entities in hierarchy or linked through pockets :)
     
  8. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    May be.
    My experience say than no one will lead those best practices :)

    You right we can debate there but almost everything that can be said, already said so we can just sit and wait for some Entities package update with timescale support if any :)

    I say Camera.main just because it looks simple but actually create huge overhead (like RandomAccess) and slow dawn code may be for 10 times and more. Such big overhead must be very explicit for programmer and my opinion is dont create such methods in core api or may be create static class SlowMethods and put it there :)

    Method with fast exit on branch will create overhead event in fast exit case :) So it is not simple decision too :)
     
  9. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    I'm a little confused by your stance on this. Do you not think such a method is required? What are we meant to do if we have something like a Target entity and need to know its timescale? Are you just assuming that that scenario will never come up?

    It would be for use alongside
    BaseSystem.GetComponent<T>(Entity)
    which is already a bit slow. I don't see that
    BaseSystem.GetTimescale(Entity entity)
    would be that much worse. The use case for it is quite rare, but sometimes necessary. Without it, correctly supporting timescales in all scenarios is impossible *. The slowness is required because it's the only way to accomplish the task. I agree that that's not ideal, but that's part of my argument against the whole feature.

    * People could make their own implementations of course, but they would be prone to bugs and inefficiencies, and it would be a higher barrier to correct use of the system. People would need a deep understanding of the timescale system's implementation to do this right. I believe API design for frameworks should be built around the pit of success, and making them work this out themselves is the opposite of that.
     
  10. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    I've understand better now :)

    For cases where logic need access to another entity timescale we need a way to access not only TimeScale component but additionally something like
    Singe EntityManager.ComputeCumulativeTimescale(Entity entity)

    and versions for use inside lambda and manual jobs alongside
    ComponentDataFromEntity<T>
    //may be
    ComputeCumulativeTimescaleForEntity)
     
    SamOld likes this.
  11. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    Yes! We're on the same page! What you're calling cumulative timescale is exactly what I was expecting my
    GetTimescale
    function to return. I'm happy we agree now!

    So just to quickly restate my concern now that we're talking the same language:

    Systems will sometimes need to call these functions in order to be correct when a target entity (meaning an entity accessed through
    ComponentDataFromEntity
    and similar) is timescaled. They have no way of only doing this when required, so it adds a cost even when the timescale feature is not being used. This means that supporting the timescale feature is expensive. People will either choose not to do so, or produce assets and game code which are slower than they need to be for the sake of conformance.

    There is also a difficulty problem. Support for timescales may often increase the complexity of the problem being solved. The cost may not only be the call to
    GetTimescale
    , but also some additional computation that is required because the maths is more complicated when timescales can't be assumed to be the same. In the simple case, it often means that the dt can't be factored out of an expression. In more extreme cases, it may actually increase the complexity class of the problem, possibly making it intractable!

    This is a design cost, too. Reasoning about interactions between entities with different timescales is hard. By including this, we raise the level of mathematical ability required to make conforming solutions to certain problems. It's not even always obvious what interactions between different timescales means - for example, what does it mean to have a slow-motion rigidbody collide with a full-motion one? Is momentum conserved? What does momentum even mean - is it defined in their local timescale or a global one? These are complex design decisions we're forcing people to make if they want their systems to conform to the standards of the ecosystem. I worry that will either push people away, or lead to messy sporadic support across systems/assets as people just ignore this feature as too hard to use.

    Here are a few classes of bugs that I expect to see:
    • Not knowing or thinking to use
      GetTimescale
      at all - it's just not an obvious thing to think about
    • Multiplying the
      ForEach
      dt into a value from a target entity - Even if the target is unscaled, if our local dt is scaled this could cause the target to be interpreted incorrectly
    • Getting the maths wrong - even if you're thinking of all of these things and trying to do it right, it's quite easy to mess up, and quite hard to test - subtle bugs could easily go unnoticed
    • Bad assumptions about other systems supporting timescales - maybe System A assumes System B has already accounted for timescaling, but it hasn't - this increases coupling between systems
    • Ambiguous documentation on level of support - this is a bug because when the spec isn't clear, behaviour cannot be said to be conforming to it
    • Assets that appear to automatically support timescales in most scenarios, but then break in one edge case a year down the line and cause hell - the algorithmic complexity jump of fixing this could be large
    • Divide-by-zero errors when timescale is used to pause the game - people could easily forget that dt may be zero
    • Incompatible design interpretations of what interactions between different timescales means - System A may assume it means one thing, but System B may assume it means another

    In my mind, there are so many ways that this could cause headaches that it's just not worth it, no matter how cool it would be to be able to do these things. Design solutions are welcome, as I would love to have this feature, just not with these problems.

    Sorry for restating all of this yet again, I should probably stop going on about it at some point!
     
  12. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    One conceptual solution to some of these worries might be to allow entity queries to somehow filter on other entities referenced by components. To illustrate:

    Code (CSharp):
    1.  
    2. Entities.WithSharedTimescale(Target target => target.entity).ForEach((float deltaTime, Target target) => {
    3.     // Code that relies on timescales being the same
    4.     // Doesn't have to call GetTimescale on target.entity
    5.     // Makes assumptions in the maths, reducing the problem complexity
    6. });
    7.  
    8. Entities.WithDifferentTimescale(Target target => target.entity).ForEach((float deltaTime, Target target) => {
    9.     // More expensive code path for when the assumption can't be made
    10. });
    11.  
    The timescales would still need to be accessed by the engine, but the user code could make safe assumptions. Yes it's adding a random memory access to the query which is bad, but the user would have had to do that in the body anyway. I don't know how well this could be made to play with parallelism and vectorisation. I'm trusting the ECS team and Burst to work their magic on this, and hand-waving the problem for now. This is just a first stage conceptual idea - maybe the performance hurdle will fell it.

    This would mean that the level of support would be clearly expressed in the queries, which I find a bit better. It's not perfect, but it's cleaner. It's also self-documenting without having to look at asset code, as queries made by systems are displayed in the editor tooling.

    It could be quite well enforced by the compiler. A warning could be given when a value obtained via
    SystemBase.GetComponent
    is combined (generally meaning multiplied) by dt, but without either
    WithSharedTimescale
    or
    WithDifferentTimescale
    being specified. That way, users are forced to confront the problem when it comes up in building the system. Nobody would ever forget to think about it, and they would clearly document the behaviour they're expecting.

    The engine could even give a warning when a query fails because of
    WithSharedTimescale
    and there's no
    WithDifferentTimescale
    equivalent. Then we would know immediately when a timescale change is unsupported by some part of our codebase.

    Thoughts?
     
    JesOb likes this.
  13. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    To clarify scenarios:

    Code (CSharp):
    1. // Fine
    2. Entities.ForEach((float deltaTime, ref Position position, in Velocity velocity) => {
    3.     position.value += deltaTime * velocity.value;
    4. });
    5.  
    6. // Still fine
    7. // Because deltaTime isn't being multiplied or divided into the GetComponent result
    8. // This is an imperfect rule, but should generally be right due to how dt tends to be used
    9. Entities.ForEach((float deltaTime, ref Position position, in Velocity velocity, Target target) => {
    10.     var offset = GetComponent<Position>(target.entity);
    11.     position.value += offset + deltaTime * velocity.value;
    12. });
    13.  
    14. // NOT FINE
    15. // deltaTime being multiplied by the result of GetComponent
    16. // Dangerous if timescales aren't the same
    17. // Reasonable rules for dangerous patterns could make the engine throw a warning here
    18. Entities.ForEach((float deltaTime, ref Position position, Velocity velocity) => {
    19.     var offset = deltaTime * GetComponent<Velocity>(target.entity);
    20.     position.value += offset + deltaTime * velocity.value;
    21. });
    22.  
    23. // Fine again
    24. // The query now explicitly specifies intent
    25. // The engine assumes that this is a considered decision so allows it
    26. // As part of the query, this condition will now be displayed in editor UI, making the level of support clear
    27. // The code can make mathematical simplifications by assuming the timescale is shared
    28. Entities.WithSharedTimescale(Target target => target.entity).ForEach((float deltaTime, ref Position position, Velocity velocity) => {
    29.     var offset = deltaTime * GetComponent<Velocity>(target.entity);
    30.     position.value += offset + deltaTime * velocity.value;
    31. });
    32.  
    33. // Fine
    34. // This properly accounts for the different timescales
    35. // Only runs this more complex path when needed
    36. // In real code, the maths on this path would often be significantly more involved
    37. Entities.WithDifferentTimescale(Target target => target.entity).ForEach((float deltaTime, ref Position position, Velocity velocity) => {
    38.     var offset = GetTimescale(target.entity) * GetComponent<Velocity>(target.entity);
    39.     position.value += offset + deltaTime * velocity.value;
    40. });
    41.  
    42. // Also fine
    43. // Now this is multiplying the target entity velocity by our local timescale
    44. // That seems a little weird, but the user has explicitly specified their intent
    45. // Trust that they know what they're doing
    46. Entities.WithDifferentTimescale(Target target => target.entity).ForEach((float deltaTime, ref Position position, Velocity velocity) => {
    47.     var offset = deltaTime * GetComponent<Velocity>(target.entity);
    48.     position.value += offset + deltaTime * velocity.value;
    49. });
    The failure modes are now cleaner. If support for timescales is not present and intended in a system, the whole system will simply not run. This is better than it running with a subtle bug. The engine can warn the user if this happens, so the bug won't go unnoticed.

    It changes nothing about more normal systems. Support is still automatic for them.

    There are still complex messy bugs that can arise, but I believe this reduces the size of that problem quite significantly.
     
    Last edited: May 9, 2020
    JesOb likes this.
  14. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    Just another 2 cents :)

    I have agree with you that there about potential bug places.
    There definitely will be cases with subtle bugs or cases where bugfix will lead to big rewrite.

    But :)
    • This is for projects that want to use timescale. All other projects - sleep in peace :)
    • Project that will use timescale for global things like different timescale for UI vs gameworld vs ghost pets mostly everything will be OK
    • For project that want to use timescale inside gameplay things there will be 80/20. But dealing with timescale in gameplay is hard by default so 80% of cases that supported by default is big win. Yes 20% of code can become hell :)
     
  15. SamOld

    SamOld

    Joined:
    Aug 17, 2018
    Posts:
    333
    It's for any module that could potentially need to interact with another module that uses timescale. That means basically all assets, because the consumer may be using timescales.

    Following the principles of good modular software engineering, separation of concerns, and avoidance of dependencies it means pretty much every single system. System decoupling is one of the things that makes ECS so nice. Reintroducing a form of coupling with the implicit assumptions made here is bad. That would be the framework and the engine encouraging/forcing users to engage in bad software engineering practices. Unity's done that enough already over the years, let's not go there again in our brave new DOTS world.

    I consider that unacceptable from the perspective of the framework designer and ecosystem curator. That's where our philosophies differ.

    I do think that my query proposal may be a good enough solution. I think I could probably support this feature with that in there because while perils remain, they're much reduced.
     
  16. JesOb

    JesOb

    Joined:
    Sep 3, 2012
    Posts:
    1,105
    I think we state our points enough :)

    Exact decision is in UT hands, Entites 11 was out and looks like they in stabilization phase :)

    @Joachim_Ante can you add some notes about plans for it and if any than roughly when :)
    Thanks :)

     
    SamOld likes this.