Simplify agent spawning

[reeseschultz]

Quoting @kobechenyang:

I also found out once I change the translation value Y of the surface. Somehow it breaks something. I set the surface Y to 100 and using agent offset to be 101 so it spawns correctly. But somehow it didn't find the target surface. Is that how I should use the offset value?

[reeseschultz]

I see what's going on. There are two issues:

1. The initial destination height needs to be set accordingly (to 100 specifically in this case). Render bounds height for getting a random position within will always approach zero here since it's local. So you can just add a new float3(0, 100, 0) to the output value of NavUtil.GetRandomPointInBounds or capture it as a variable dest and say dest.y = 100.
2. But even if you account for the above, having the agent.Offset.y at 101 will, well, offset the agent 101 units above the surface, meaning the agent will be moving around in the sky above.

That render bounds helper function in NavUtil, and the agent.Offset basically work as intended, but the problem you're encountering has to do with the need to set an initial world position on the agent. agent.Offset is relative to the surface, but it works relative to the world on scene start, and there's no alternative to setting that initial world position.

Tomorrow I'll add a field to the agent to set that initial world position from which the agent can raycast to detect a surface below, then your agent.Offset should go back to (0, 1, 0), at least if you're modifying the performance demo. I may additionally update the docs to clarify this.

[reeseschultz]

Wait a second, sorry. I wasn't thinking.

You should just be able to just set the initial Translation or LocalToWorld on the agent when spawning to resolve the second point I raised above. No point in me duplicating that as a field in the agent.

Let me know if that works for you.

[kobechenyang]

Hi Reese Thanks for looking into this. I have set Translation to float3(0,101,0) and it didn't help me. Agents still spawning at origin and parent is null. As you can see in my screenshot. My LocalToParent has a Y value of 101 and Translation has a Y value of 101. But the LocalToWorld doesn't have a value of 101.

[reeseschultz]

@kobechenyang I've a solution for you, but let me explain what's going on with the Translation first since it tripped me up as well.

Problem

I thought I could initially set the Translation on spawned agents since it's basically just shorthand for setting the position on the LocalToWorld when there is no parent; however, when spawning agents they require the Parent and LocalToParent components so the nav system can set them later. The problem is that, in so doing, the parent can be Entity.Null initially, and, if so, setting the Translation as such seems to behave like a no-op instead of just writing to LocalToWorld. So Translation is only shorthand for setting the LocalToWorld position when there are no parent components, not when Parent.Value is Entity.Null.

Unfortunate—it seems the default for the TransformSystem could be improved if I'm right about that. Still, ideally I could move the Parent and LocalToParent component adding out of user code. Then setting the Translation should work as expected, I think. Alternatively, users could be able to set the parent components only if they know what the Parent.Value surface ought to be. I'll experiment and see how I can simplify agent initialization from a user perspective. If I accomplish something to that end, I'll send you a message with the release associated with the change.

Workaround

1. When spawning agents, set their LocalToWorld like so:

new LocalToWorld {
  Value = float4x4.TRS(
    new float3(0, 101, 0),
    quaternion.identity,
    1
  )
}

This will initialize the world position as expected, guaranteed, but it's a bit wordy. Like I said, if I can take the Parent and LocalToParent out of users' hands, then just setting the Translation initially would be preferable.

2. The other thing, which you know but I'll quote for anyone reading:

The initial destination height needs to be set accordingly (to 100 specifically in this case). Render bounds height for getting a random position within will always approach zero here since it's local. So you can just add a new float3(0, 100, 0) to the output value of NavUtil.GetRandomPointInBounds or capture it as a variable dest and say dest.y = 100.

It seems like including an optional offset parameter for NavUtil.GetRandomPointInBounds would be a nice-to-have as well, taking another look at this.

Conclusion

Let me know if that works. Please keep the issues coming if you find anything else. I appreciate that you're battle-testing this nav package and communicating with me. I'll keep this issue open until related improvements are made, and/or updates to the docs to clarify this process.

[reeseschultz]

The referenced commit, fd51ad3, removes the need for users to manually add the parent components, meaning setting the Translation directly writes to the LocalToWorld position on initialization, which is much more idiomatic. I'll release that along with another change or two later today, probably.

[reeseschultz]

Eh, I think that'll do it. Spawning agents is much more straightforward now:

SpawnSystem.Enqueue(new Spawn()
    .WithPrefab(prefabEntity)
    .WithComponentList(
        new NavAgent
        {
            JumpDegrees = 45,
            JumpGravity = 200,
            TranslationSpeed = 20,
            TypeID = NavUtil.GetAgentType(NavConstants.HUMANOID),
            Offset = new float3(0, 1, 0)
        },
        new NavNeedsSurface { },
        new Translation
        {
            Value = new float3(0, 101, 0) // This should work as you expect now.
        }
    ),
    enqueueCount
);

I'll do another release.

[kobechenyang]

Hi, Reese Thanks for making changes so quick ! Unfortunately after I tried the new version. It still not works correctly. I have set the surface height to 100. Then use this code to spawn

SpawnSystem.Enqueue(new Spawn()
    .WithPrefab(prefabEntity)
    .WithComponentList(
        new NavAgent
        {
            JumpDegrees = 45,
            JumpGravity = 200,
            TranslationSpeed = 20,
            TypeID = NavUtil.GetAgentType(NavConstants.HUMANOID),
            Offset = new float3(0, 1, 0)
        },
        new NavNeedsSurface { },
        new Translation
        {
            Value = new float3(0, 101, 0) 
        }
    ),
    enqueueCount
);

As you can see in my screen shot. LocalToParent also has a Y value of 101 and LocalToWorld Y value is 201. My guess it is adding the translation value of parent?

If I do

 new Translation
 {
      Value = new float3(0, 1, 0) 
 }

I am spawning from origin again. If I am understanding correct here, translation is the world position and you don't need to add the parent translation?

Thanks !

[reeseschultz]

I think I had temporary interop compounding with non-deterministic system order cached that produced a working outcome earlier, because now that code doesn't work for me either. Argh! I'm sorry about that.

This should work for sure, but it's a bit longer:

SpawnSystem.Enqueue(new Spawn()
    .WithPrefab(prefabEntity)
    .WithComponentList(
        new NavAgent
        {
            JumpDegrees = 45,
            JumpGravity = 200,
            TranslationSpeed = 20,
            TypeID = NavUtil.GetAgentType(NavConstants.HUMANOID),
            Offset = new float3(0, 1, 0)
        },
        new Parent { },
        new LocalToParent { },
        new NavNeedsSurface { },
        new LocalToWorld
        {
            Value = float4x4.TRS(
                new float3(0, 101, 0),
                quaternion.identity,
                1
            )
        }
    ),
    enqueueCount
);

Even if I try to force the package's NavSurfaceSystem to update after the TransformSystemGroup, something's still not right. So for now, just stick with this workaround. I'm gonna have to experiment more to make agent initialization easier. This could potentially be a problem with the TransformSystem, which Unity maintains, but I'm not sure right now. I'll need some time to deliberate.

FYI, Translation is only supposed to write directly to the LocalToWorld position when there is no parent. Think of it this way: if you have no parent, then your transform must be relative to the world; but if you have a parent, then your position must be relative to said parent. Unity does actually have a way to hook in and change how transforms are handled, like if you wanted to change multiplicative order of matrix operations, but that's not a road I want to go down if I can avoid it. My feeling is this is a simpler system order issue, but I could be wrong.

[kobechenyang]

Hi Reese. Thanks for you reply. I indeed found that ecs transform system is very painful to work with. What would be a good way to get the NavAgent's world position and doing some lerp animations? Should I do set localToWorld matrix directly or transform the animation to local translation and change translation values?

[reeseschultz]

The ECS transform system has its quirks, but I empathize with the maintainers because they must contend with requirements I would have little patience for.

What would be a good way to get the NavAgent's world position and doing some lerp animations? Should I do set localToWorld matrix directly or transform the animation to local translation and change translation values?

You need to read the docs on the transform system and write some code testing it, and your assumptions. I can tell you that NavAgents are local to a parent surface after it's initially set. Modifying their Translation applies in terms local to the parent if it exists, and modifying the LocalToWorld modifies in terms local to the world. When an entity has an associated and populated Parent.Value, modifying the Translation is normally what you want to do for interpolation.

[reeseschultz]

I'm closing this, because I think the established convention established in the demos is fine the way it is. It guides the user to define the position of an agent in the most explicit terms, which is of the utmost importance when there's parent transformation going on.