UnityTechnologies / open-project-1

Unity Open Project #1: Chop Chop
http://unity.com/open-projects
Apache License 2.0
5.73k stars 2.03k forks source link

Unable to set player's spawn location #197

Closed mtakac closed 3 years ago

mtakac commented 3 years ago

Short description SpawnSystem ignores changes in spawn location.

Expected behaviour First of all, my apologies if I am just missing something. However based on a brief scan of the source code, I would expect, that if I change the position of SpawnSystem location, the player will spawn at this new position.

This is not the case. The player always spawns at the default (0, 0, 0) position. The SpawnSystem.cs script is passing the correct position to the InstantiatePlayer method so the problem must be somewhere else.

To Reproduce Steps to reproduce the behavior:

  1. Go to TestingGround scene
  2. Change the position of Location 01 (child of SpawnSystem game object)
  3. Hit play
  4. Player spawns at position 0, 0, 0, ignoring the position you set in SpanSystem's location
mtakac commented 3 years ago

It seems like the issue is caused by animations. If I disable Animator component on the PigChef prefab the SpawnSystem spawns player at the correct position.

mtakac commented 3 years ago

One more observation: it seems like enabling the Apply Root Motion option in Animator component panel on PigChef prefab fixed the issue and didn't break the animations. I am not really sure why or if this a good fix. Let me know, I am happy to open a PR if this solution is good enough.

ciro-unity commented 3 years ago

Sounds like an interesting thing that we definitely need to fix, but also understand why it happens and how can we prevent it. I know that some animation clips have keyframes on position and rotation for the root, which generally I'd like to avoid.

Also a little heads-up. There's a setting in the Animator which can feel quite mystical :D called "Write Defaults". A short explanation.

Having it on/off can change a lot how a state machine behaves. It's all logical, but you need to be aware of its value (and each state can have a different value for it).

Let me know if it's not clear, I'll re-explain :)

And no PR needed for now, but I'll keep the issue open. Thanks a lot for it, @mtakac !!

mtakac commented 3 years ago

Thanks for the explanation @ciro-unity. I didn't know about Write Defaults option. Sounds like it could come handy to be aware of it.

However I don't think this is related to this exact issue. If I understood correctly, Write Defaults option is relevant when destroying and the re-instantiating animated object, but that doesn't seem to be the case here. Also I verified that toggling it on various animation clips doesn't fix the issue, but I think you're right about the problem being in animations setting the position:

Screenshot 2020-11-19 at 03 02 20

Unfortunately, I don't know enough about 3D animations in Unity to do anything about it, but would be interesting to see what happens if somebody updates animations to remove the position.

Wodopo commented 3 years ago

While checking Apply Root Motion allows the character to spawn at the desired location and walk around it's not without problems.

Changing settings on the Animator controller while playing, and with the Apply Root Motion off gets the desired behaviour, but entering Play Mode with the same settings gets a different, undesired, behaviour.

A video to demonstrate: https://youtu.be/DDoaKjf54KI

jandd661 commented 3 years ago

Greetings, I'm new to the project. After looking things over I noticed the animation(s) "keyframe" the position of the character. Removing the position keyframes from the animations (by duplicating them, then removing the position attribute, then retargeting the animator) has resolved this issue in my testing.

I am reluctant to call this a fix as I don't know what the big picture is for the animations. Currently, the animations are baked into the model and that is not my area of expertise.

ciro-unity commented 3 years ago

Thanks everyone for chiming in. I have asked the 3d modeler to remove the keys from the model's root, she provided me with new models which will soon be integrated on main and art-assets, which should fix this issue.

ErikBehar commented 3 years ago

this issue can be closed now

ciro-unity commented 3 years ago

Thanks! And yes, we implemented the fix as described by @jandd661