miwarnec / DOTSNET

Public DOTSNET issue tracker
20 stars 0 forks source link

SpawnMessageSystem does not consider the execution order when spawning #12

Closed gabrjt closed 4 years ago

gabrjt commented 4 years ago

Although SpawnMessageSystem belongs to a Group that executes before the TransformSystem group and the PresentationGroup, client.Spawn() method is being called in the OnMessage callback. So even though the message transform data is correct, the entity spawns and renders with default transform values for one frame before being set to its correct transform values.

Steps to reproduce:

  1. Spawn entities with transforms that does not have the default values.
  2. Observe the entities being spawned in the Scene view at the origin before being teleported to their correct location. (May be hard to notice, they just blink at the origin).

A simple way to solve this is caching the spawn messages and processing them OnUpdate. This way they will be spawned in the correct group respecting the execution order.

using Unity.Collections;
using Unity.Entities;

namespace DOTSNET
{
    // use SelectiveAuthoring to create/inherit it selectively
    [DisableAutoCreation]
    public class SpawnMessageSystem : NetworkClientMessageSystem<SpawnMessage>
    {
        NativeList<SpawnMessage> _list;

        protected override void OnCreate()
        {
            base.OnCreate();

            _list = new NativeList<SpawnMessage>(Allocator.Persistent);
        }

        protected override void OnDestroy()
        {
            base.OnDestroy();

            if (_list.IsCreated)
            {
                _list.Dispose();
            }
        }

        protected override void OnUpdate()
        {
            for (var index = 0; index < _list.Length; index++)
            {
                var message = _list[index];

                client.Spawn(
                    message.prefabId,
                    message.netId,
                    message.owned != 0,
                    message.position,
                    message.rotation
                );
            }

            _list.Clear();
        }

        protected override void OnMessage(SpawnMessage message)
        {
            _list.Add(message);
        }
    }
}
miwarnec commented 4 years ago

nice find, thanks. will check it out.

miwarnec commented 4 years ago

hey @gabrjt , just so I understand: SpawnMessage already contains and applies the initial position. but they still have the wrong position? as if ECS SetComponentData(Transform) is not applied until a frame later?

gabrjt commented 4 years ago

Yes, I believe that the OnMessage callback is invoked after the TransformSystemGroup, so even if the entity have non default Translation and Rotation values, they will only be applied in the next frame. Until then, LocalToWorld has the default value and that's why meshes are rendered at the World origin since the PresentationGroup comes after the callback and uses LocalToWorld in order to perform the rendering.

Since the callback probably also happens after the Physics systems, they would not collide or be queried in the frame they were instantiated, but maybe they would collide at the origin before applying the values since the Physics systems runs before the TransformSystemGroup 😬 (not sure, haven't tested this case, don't remember if Physics uses Translation/Rotation or LocalToWorld)

gabrjt commented 4 years ago

Closing this because there's more thing involved and this is not the real solution for the problem.

So I had the Interest Management System running before every other system that send messages on the server, but the problem still occurred despite the solution that I've previously posted (I thought it seemed to work for me, but then it started to happen again).

What really solved for me was separating the (ClientConnected/ServerActive)SimulationSystemGroup into three groups.

One for sending messages (that I've called WriteMessageSystemGroup), another for receiving messages (ReadMessageSystemGroup) and one for game logic simulation, the NetworkSimulationSystemGroup. The ClientConnectedSimulationSystemGroup and ServerActiveSimulationSystemGroup still exists, but they do not contain any systems (yes, I've refactored everything regarding system groups 😅 ). They now just enable/disable these new system groups.

ReadMessageSystemGroup updates before ApplyPhysicsGroup. NetworkSimulationSystemGroup updates in the ApplyPhysicsGroup. And the WriteMessageSystemGroup updates after the EndFramePhysicsSystem (also after the TransformSystemGroup). This means that messages sent were already processed by physics and transform systems, and received messages are ready to be processed by game logic simulation.

The issue before was that messages were being sent while in the middle of the simulation, so they didn't correspond to the positions they should be after the physics simulation when sending messages...

However, this adds complexity and is something that maybe should not be enforced by the library (for now at least). Those read/write groups are also not running in FixedUpdate, so there must be a buffered solution for handling inputs.

With all that said, I've maintained the strategy to cache received messages because it's clearer for me when the system is really processing the messages OnUpdate instead of OnMessage. And regarding system groups, I liked the NetworkSimulationSystemGroup instead of using ClientConnectedSimulationSystemGroup for client game logic systems and ServerActiveSimulationSystemGroup for server game logic systems. This allows a system to execute both in the client and in the server without having to make it abstract and creating stubs for each world to run on each simulation system group. Useful for client side prediction!