miwarnec / DOTSNET

Public DOTSNET issue tracker
20 stars 0 forks source link

DOTSNET relies too much on it's own prefab system #2

Open LuisEGomezV opened 4 years ago

LuisEGomezV commented 4 years ago

Problem DOTSNET have a built-in prefab system which is used by NetworkClientSystem among others. While it is an appreciated feature, it is the only way to spawn entities in network. So if for example a project requires a lot of different prefabs to be spawned, it makes things super complicated.

Solution 1

As mentioned before, NetworkClientSystem uses the prefab system to spawn entities, and after instantiating it run some methods to register the entity so it can be used by other systems.

So to solve the problem, NetworkClientSystem can simply expose a method

void OnSpawned(Entity entity, ulong netId, bool owned)

in which it makes the registration process, without actually spawning the entity, leaving that responsibility to the user.

Solution 2

Another solution, a more complicated one, could be to make the prefabs system a more flexible one, where the user can decide where the prefabs came from, and when. Using Addressables, for example.

vipvex commented 4 years ago

I agree with this. Solution #1 is something I would really like to have. The game I'm building is build with pure ECS -- not GameObjects or prefabs of any kind. And while it is nice to have the conversion workflow it actually makes it more difficult for projects that do pure ECS.

OnSpawned(Entity entity, ulong netId, bool owned) +1

vipvex commented 4 years ago

Another thing I just realized is that it's a bit tricky to spawn a plain entity without any Translation, Rotation, Scale components. I use a number of entities that don't need to show up in the game at all. Entities that just store data and organize things like Civilization, Faction, Team etc. But with the current workflow I would have to convert the GameObject to an entity which forces those entities to have Translation, Rotation, Scale components etc.

LuisEGomezV commented 4 years ago

Indeed, the provided InterestManagementSystem (BruteForce) will ignore entities without Translation or Rotation, so they will not be spawned automatically to the clients. Of course you can send a message manually to spawn on ever client you need, but there is no built-in way

TopherMcKnight commented 4 years ago

the current InterestManagementSystem will ignore entities without Translation

I feel this should be corrected to the provided interest management system. The base InterestManagementSystem doesn't seem to have any issues or requirements on translation.

You could just as easily add an extra loop in the currently existing BruteForceInterestManagementSystem or remove the translation requirement in the entities.foreach portion of the code.

LuisEGomezV commented 4 years ago

I feel this should be corrected to the provided interest management system. The base InterestManagementSystem doesn't seem to have any issues or requirements on translation.

Yes, this isn't actually the main issue, just a comment. Actually in the vipvex case it could be even better not to rely on any InterestManagementSystem to update his example entities (Civilization, Faction, Team etc.) but instead manually spawn them.

But yes, I'm editing that. Thanks

miwarnec commented 4 years ago

Hey guys. Would this: https://github.com/vis2k/DOTSNET/issues/6 be enough once it's done? (Archteypes)

LuisEGomezV commented 4 years ago

Not really, the need here, is to make networkable entities that are already spawned by different ways, other than the prefab system. And to make networkable those entities need to be registered in the hashmap inside NetworkClientSystem and NetworkServerSystem so a method to register and unregister entities is needed

LuisEGomezV commented 4 years ago

Actually this is very simple, I have this working on NetworkClientSystem.

public void OnSpawned(Entity entity, ulong netId, bool owned)
{
    spawned[netId] = entity;
    // reuse ApplyState to apply the rest of the state
    ApplyState(netId, owned);
}

public void RemoveFromNet(ulong netId)
{
    // find spawned entity
    if (spawned.TryGetValue(netId, out Entity entity))
    {
        // remove from spawned
        spawned.Remove(netId);
        EntityManager.RemoveComponent<NetworkEntity>(entity);
    }
}
gabrjt commented 4 years ago

My two cents on the Prefab System.

The main thing I've missed so far is a way to specify different prefabs for the Server and the Client (Proxy/Owner when applicable).

This could be done maybe by deferring the prefab instantiation. The GUID -> Bytes16 map could be applied to a prefab which points to other prefab versions (server, client proxy/owner).

Why do I miss this feature? Because sometimes I want distinct archetypes for the server and the client (proxy or owner).

So far I've been using conditionals when converting in order to avoid conversion to an undesired world. For example if (dstManager.World != Bootstrap.ClientWorld) return;

Another script that I use on child GameObjects that will be converted is like:

public class TargetWorldConversionAuthoring : MonoBehaviour, IConvertGameObjectToEntity
{
    public TargetWorld Value;

    public void Convert(Entity entity, EntityManager dstManager, GameObjectConversionSystem conversionSystem)
    {
        if (Value == TargetWorld.Both ||
            Value == TargetWorld.ClientWorld && dstManager.World == Bootstrap.ClientWorld ||
            Value == TargetWorld.ServerWorld && dstManager.World == Bootstrap.ServerWorld)
        {
            return;
        }

        dstManager.DestroyEntity(conversionSystem.GetPrimaryEntity(gameObject));
    }
}

So with this its possible to have a prefab with a child GameObject that has a mesh and that will be converted only in the client.

So far I've been able to reuse all my prefabs in the Server/Client and do relevant transforms for a specific instance in the owner when applicable (like instantiating a camera, no need to instantiate cameras for the proxies instances), but maybe with an integrated Prefab System this could be more straight forward.

Another think that I've noticed, Dictionary with blittable types is being used instead of NativeHashMap. Any reason for this? I'm pretty sure some performance could be gained in Systems where you need to check if the PrefabId exists in the Dictionary if it was a NativeHashMap, and this can also open doors for batch instantiating prefabs :)

LuisEGomezV commented 4 years ago

@gabrjt I think that should be moved to it's own issue, because if not, it could get lost in the comments. The problem here is that there is no "official" way to spawn things without using the built-in PrefabSystem, but it has a lot of limitations, so some users will want to use their own "Prefab systems". So the requirement is just to be able to do that without having to modify DOTSNET code. The solution is actually very simple, I don't know why @vis2k has not implemented it yet.