Search Unity

  1. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

AddInputDependency() issues if used on the first frame?

Discussion in 'Physics for ECS' started by PhilSA, Aug 6, 2020.

  1. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    I am getting this error spammed constantly during play mode:
    And here is the OnUpdate() of the CharacterMovementSystem:
    Code (CSharp):
    1. protected override void OnUpdate()
    2.         {
    3.             Dependency = new CharacterMovementJob
    4.             {
    5.                 DeltaTime = Time.DeltaTime,
    6.                 PhysicsWorld = BuildPhysicsWorldSystem.PhysicsWorld,
    7.                 (....... more stuff),
    8.             }.ScheduleParallel(CharacterQuery, Dependency);
    9.  
    10.             // Make BuildPhysicsWorld wait for this job
    11.             BuildPhysicsWorldSystem.AddInputDependency(Dependency);
    12.         }
    I would've expected this setup work without any problems due to the "BuildPhysicsWorldSystem.AddInputDependency" I do at the end, but looks like that doesn't do it.

    What's weird is that if, during Play mode, I disable my CharacterMovementSystem in the Entity Debugger and re-enable it, the errors stop and everything starts working again. Almost as if this was a first-frame error that caused errors to repeat every frame. EDIT: I did a test and I can confirm the error doesn't pop up if I skip scheduling the job on the first few frames

    Using:
    • Unity 2020.2a19
    • Unity Physics 0.4.1-preview
    • Entities 0.13.0-preview.24
    • Jobs 0.4.0-preview.18
     
    Last edited: Aug 6, 2020
    Tony_Max likes this.
  2. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,356
    You need to combine with the physics dependency and set it back. You are telling physics about your dependency but you haven't told your own system about physics. So it's going to work either sporadically or not at all just depending on how you have everything ordered.

    Also if your stuff runs after build you need to set back against end frame.

    A reliable setup we have used for a long time is this:

    Code (csharp):
    1.  
    2. [UpdateInGroup(typeof(SimulationSystemGroup))]
    3.     [UpdateAfter(typeof(Unity.Physics.Systems.BuildPhysicsWorld))]
    4.     [UpdateAfter(typeof(Unity.Physics.Systems.ExportPhysicsWorld))]
    5.     [UpdateBefore(typeof(Unity.Physics.Systems.EndFramePhysicsSystem))]
    6.     [UpdateBefore(typeof(TransformSystemGroup))]
    7.     public class GameLogicGroup : ComponentSystemGroup
    8.     {
    9.     }
    10.  
    And we have a system with these methods where before any jobs that use physics we combine and after we schedule we set back.

    Code (csharp):
    1.  
    2. public JobHandle Combine(JobHandle other)
    3.         {
    4.             return JobHandle.CombineDependencies(BuildPhysicsWorld.GetOutputDependency(), other);
    5.         }
    6.  
    7.         public void Set(JobHandle other)
    8.         {
    9.             EndFramePhysicsSystem.AddInputDependency(other);
    10.         }
    11.  
     
    Tony_Max and PhilSA like this.
  3. petarmHavok

    petarmHavok

    Joined:
    Nov 20, 2018
    Posts:
    461
    Just to add to the previous reply, adding the input dependency to BuildPhysicsWorld makes sense if your system is scheduled before BuildPhysicsWorld, if it is after you need to add dependency to EndFramePhysicsSystem, that will implicitly make the next BuildPhysicsWorld wait for it.
     
  4. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    Here's what my whole system looks like now (simplified)
    Code (CSharp):
    1. [UpdateBefore(typeof(BuildPhysicsWorld))]
    2. public class CharacterMovementSystem : SystemBase
    3. {
    4.     public EndFramePhysicsSystem EndFramePhysicsSystem;
    5.     public BuildPhysicsWorld BuildPhysicsWorldSystem;
    6.     public EntityQuery CharacterQuery;
    7.  
    8.     protected override void OnCreate()
    9.     {
    10.         EndFramePhysicsSystem = World.GetOrCreateSystem<EndFramePhysicsSystem>();
    11.         BuildPhysicsWorldSystem = World.GetOrCreateSystem<BuildPhysicsWorld>();
    12.         CharacterQuery = GetEntityQuery(CharacterUtilities.GetCharacterEntityQueryDesc());
    13.     }
    14.  
    15.     protected override void OnUpdate()
    16.     {
    17.         Dependency = JobHandle.CombineDependencies(EndFramePhysicsSystem.GetOutputDependency(), Dependency);
    18.  
    19.         Dependency = new CharacterMovementJob
    20.         {
    21.             DeltaTime = Time.DeltaTime,
    22.             PhysicsWorld = BuildPhysicsWorldSystem.PhysicsWorld,
    23.             // ......
    24.         }.ScheduleParallel(CharacterQuery, Dependency);
    25.  
    26.         // Make BuildPhysicsWorld wait for this job
    27.         BuildPhysicsWorldSystem.AddInputDependency(Dependency);
    28.     }
    29. }  
    • updates before BuildPhysicsWorld
    • (not shown) the PhysicsWorld passed to the job is [ReadOnly]
    • Combines dependency with EndFramePhysics before scheduling
    • Adds input dependency to BuildPhysicsWorld after scheduling
    But I'm still getting the error every frame, unless I skip scheduling the job on the first frame; then I never get the error and everything works
     
  5. petarmHavok

    petarmHavok

    Joined:
    Nov 20, 2018
    Posts:
    461
    Maybe irrelevant to this problem, but what is the point of scheduling the job in the first frame before BuildPhysicsWorld, since PhysicsWorld is not valid at all at that point? Maybe if you guarded your job with something like if (PhysicsWorld.NumBodies > 0) Schedule(); ?
     
  6. petarmHavok

    petarmHavok

    Joined:
    Nov 20, 2018
    Posts:
    461
    Here is a theory about what happens: you schedule your job that reads from m_Bodies array. Your job runs. BuildPhysicsWorld runs the OnUpdate() where it calls PhysicsWorld.Reset() which deallocates old and creates new m_Bodies array (first frame that there are bodies present). And it crashes, because it is deallocating array you are trying to read from.
     
  7. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,926
    I gave this a try, but that didn't solve it.

    I need this to run before physics because I have options where the character moves with PhysicsVelocity (as a kinematic or dynamic body, so it deals better with pushing other bodies), and so it would always be one frame late if I ran this after physics. Basically:
    - ColliderCast for collisions
    - determine final target position
    - set a PhysicsVelocity that would bring us to that position over next fixedUpdate
    - Build + Step physics

    However I'm realizing now that this is also "one frame late" in a way, because of the PhysicsWorld not being up to date. I think what I'd really need for true frame perfection is:
    - Build+Step physics world
    - Character collisions job and set a PhysicsVelocity
    - Build+Step physics world a second time, and Export

    That makes sense. I think I was assuming the BuildPhysicsWorld's resetting of PhysicsWorld would be done in a job that takes the InputDependencies, but looking at the code I see that this is not the case.

    ....but now I don't even understand why I don't get the error with the first-frame-skip approach.
     
    Last edited: Aug 6, 2020
  8. romeo_ftv

    romeo_ftv

    Joined:
    Oct 20, 2009
    Posts:
    36
  9. petarmHavok

    petarmHavok

    Joined:
    Nov 20, 2018
    Posts:
    461
    And to reply about the PhysicsWorld.Reset(). It's not done via a job since it does reallocation of persistent memory. We tried a couple of times but didn't come up with a better solution.
     
  10. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,356
    I decided to move our character controller to before BuildPhysicsWorld and it errors out every frame on reset. And there is no dependency you can set that fixes it. end frame has already been combined, build input dep isn't completed so has no effect. You can track the dependencies involved yourself and complete them, probably the best solution for now. And I think generally you should have abstractions around the whole combine/set back flow anyways. So if your flow changes you don't need to change dozens of systems. You are probably going to have a different set of dependencies you combine/set for your physics logic and the rest of your casting outside of the fixed step group also. Another reason to abstract out the combine/set stuff.