miwarnec / DOTSNET

Public DOTSNET issue tracker
20 stars 0 forks source link

NetworkEntity needs changes #20

Open LuisEGomezV opened 4 years ago

LuisEGomezV commented 4 years ago

Current Usage

public struct NetworkIdentity : IComponentData
{
    public ulong netId;

    public Bytes16 prefabId;

    public int? connectionId;
    public bool owned;
}

With this approach, the following problems are present:

  1. Not every networked entity needs a prefab Id. There are certain cases where the networked entities are not spawned using this giant prefab id, but instead uses a custom message, or whatever the user need.

  2. Not every networked entity has "connectionId". This is already stated by it's current usage, but still always there.

  3. Not every networked entity has "owned". This is already stated by it's current usage, but still always there.

  4. "connectionId" and "owned", could be used to filter entities if they exists as separated components.

Proposal

public struct NetworkIdentity : IComponentData
{
    public ulong netId;
    public static implicit operator ulong(NetworkIdentity data) => data.netId;
}

This is the only required field in NetworkEntity to be able to sync data through network, the other fields just change the behavior a little, but are not required to identify a networked entity.

public struct NetworkOwner : IComponentData
{
    public int connectionId;

    public static implicit operator int(NetworkOwner owner) => owner.connectionId;
    public static implicit operator int?(NetworkOwner owner) => owner.connectionId;
    public static implicit operator NetworkObserver(NetworkOwner owner) => owner.connectionId;
    public static implicit operator NetworkOwner(int connectionId) => new NetworkOwner { connectionId = connectionId };
}

This way the server can work selectively with entities owned by clients. NetworkOwner only exists in the server. Entities without NetworkOwner are owned by the server.

public struct NetworkOwned : IComponentData
{
}

Similar to NetworkOwner, with NetworkOwned the client can work selectively with entities owned by itself. NetworkOwned only exists in the client who owns that entity.

Adding this two components in their respective worlds of course will create another archetype, but I don't it's a problem as almost every NetworkEntity will be owned by the server, So the archetype change will probably apply to certain entities that already share archetype between them (Like the players, it's pets, etc.)

Now, about NetworkPrefab. Just by removing it from NetworkEntity will be good for now. (The best will be to use a BlobAssetReference but that's for another issue.

public struct NetworkPrefab : IComponentData
{
    public Bytes16 prefabId;
}
LuisEGomezV commented 4 years ago

@vis2k Can we at least have a discussion about this?

miwarnec commented 3 years ago

sorry for late reply, been busy with Mirror :) what you proposed, is this common practice in ECS? I understand the reasoning. But at the same time, if we apply this approach to everything else then we end up in interface hell.

LuisEGomezV commented 3 years ago

I understand the reasoning. But at the same time, if we apply this approach to everything else then we end up in interface hell.

Could you elaborate on this? I don't really see why this is harmful.

shredder2500 commented 3 years ago

sorry for late reply, been busy with Mirror :) what you proposed, is this common practice in ECS? I understand the reasoning. But at the same time, if we apply this approach to everything else then we end up in interface hell.

The proposal actually follows the recommendation by unity more closely. They recommend smaller components and only having data in a component that will be used every time you query the component. Besides that if I want to do something with every network entity that has a prefabid right now i query and have a if statement where if its a seperate component its just a query. Same thing for local play and etc

Morbeavus commented 3 years ago

Just to pitch in this is from the DOTS best practices: https://learn.unity.com/tutorial/part-3-2-managing-the-data-transformation-pipeline?uv=2020.1&courseId=60132919edbc2a56f9d439c3&projectId=6013255bedbc2a2e590fbe60#60132f40edbc2a56f9d43ac0

I would vote for the proposed change since the pros outweigh the cons(ownership usually does not change often, we save byte on all entities since bool is replaced with just tag which is "free", we can more effectively query for localOwner/nonLocal, do work only on the querried entites, etc...)

ghost commented 2 years ago

@vis2k can you elaborate on the interface hell concern? I'd really like to avoid including prefab IDs entirely, if possible, for my game, because we are attaching prefabs to our synced entities another way.