libgdx/ashley

Improve support for networked games

Closed this issue · 12 comments

This issue is intended to resolve one of the remaining problems in ashley: networking. Currently, there is no built-in way to synchronize entity id's, which is useful when writing network packets. A solution to this problem would be to allow passing an entity id to addEntity(), but one issue remains: id clashes between client/server. Right now the generated id's are sequential, which means the probability of a clash is very high. I propose making it optional for an entity to carry an id, so the user has to be explicit about what entities to associate with an id. As a replacement for the current notion of "id", I suggest "index". This would be a recycled integer that can be used for storing local data (such as caches) related to an Entity outside of components.

I don't think that's a good idea. Recycling indexes will add a lot of issues, especially in network code.
If you use UDP (which you should in a game), there is no guarantee that packets arrive in order.

Now imagine this scenario:
Server creates an entity with id = 1
Client creates entity with id = 1
Server pushes a lot of packets with position updates for id = 1 (each getting a sequence id to allow ignoring old packets)
Client receives most of them, but a couple are delayed
Server deletes id = 1
Client deletes entity with id = 1
Server creates a new entity with id = 1(b)
Client creates new entity with id = 1(b)
Client receives delayed updates for 1(a) and applies them incorrectly for entity id=1.
And since updates for 1(b) have a fresh sequence id they will be ignored until they are higher than the sequence id from 1(a).

Sure there are ways to work around that issue, but generally its better to just stick with a unique 64bit id (you won't run out of ids in any scenario)
Also, overload the addEntity to allow for ids to be generated if the parameter is not passed.
You might also want to consider a scenario where the client wants to create entities with ids that the server does not have (for rendering or other stuff the server does not care about).

I have already done a Network game with Ashley and for my case, it worked fine to just map incoming entity ids to the newly created entities in a map. Since the server always tells the client which entities to destroy, the map can be kept up to date from the network code.

Indexes should not be used for network packets, as what index is assigned to a new entity is arbitrary, and can't be controlled by the user. I mentioned a new concept of IDs to be used for that purpose.

Also, overload the addEntity to allow for ids to be generated if the parameter is not passed.

Yes; I mentioned that in my post. The user is responsible for assigning ids; at least for now.

You might also want to consider a scenario where the client wants to create entities with ids that the server does not have (for rendering or other stuff the server does not care about).

Locally created entities should not be associated with an ID in the first place. Indexes should be used for that.

I have already done a Network game with Ashley and for my case, it worked fine to just map incoming entity ids to the newly created entities in a map. Since the server always tells the client which entities to destroy, the map can be kept up to date from the network code.

That works, but sounds pretty error-prone.

I guess then I don't get what the index would be for.. except for isValid checks, but you don't need an id for that a boolean would suffice.

The user is responsible for assigning ids; at least for now.

Most games don't have networking, so having to manually manage uuids is extra work for the user, even if they don't need it. Also, you would break existing games.

That works, but sounds pretty error-prone.

Since there are only two places the map is changed, there is no problem at all.

I guess then I don't get what the index would be for.. except for isValid checks, but you don't need an id for that a boolean would suffice.

The index of an entity can be used for sparse arrays storing some value related to an entity. In the future, this is also useful for storing components outside of entities, as mentioned in #87.

Most games don't have networking, so having to manually manage uuids is extra work for the user, even if they don't need it. Also, you would break existing games.

By default, entities have no ID. To assign a ID to an entity, the addEntity(Entity, long) method is used. addEntity(Entity) does not assign an ID at all.

Since there are only two places the map is changed, there is no problem at all.

I meant that it's pretty easy to forget to actually re-map the IDs, especially if you have something like Map<Long, Long>.

I'll look into improving my patch; I think the best solution is to refactor IDs into a component. Some methods will be provided by Ashley for convenience (such as Entity.getUuid()), that throws exceptions if an ID is not set. And, yes, I'll break all existing games, but other recent changes have already done so.

Any other ideas?

The index of an entity can be used for sparse arrays storing some value related to an entity. ...

So you want to create an array the size of max-integer ? Sounds like a bad idea memory-wise.
Even if you would make it resize depending on the number of entities, you still have the problem that there are games out there with a huge amount of entities.

addEntity(Entity) does not assign an ID at all.

That is the problem, because currently, every entity does have an ID. and if users want that to continue, they need to do extra work. IDs are required if you use PooledEngine and want to store a reference to an entity that might be destroyed.

I think the best solution is to refactor IDs into a component.

For what purpose ? Seems to me like there is only overhead, but no benefit.

So you want to create an array the size of max-integer ? Sounds like a bad idea memory-wise.

No; Bag can be used.

Even if you would make it resize depending on the number of entities, you still have the problem that there are games out there with a huge amount of entities.

Right now, Ashley is bad memory-wise. It stores the components of an entity in both an Array and sparse Bag. But this does not matter too much, as today's computers and VMs are capable of handling it.

That is the problem, because currently, every entity does have an ID. and if users want that to continue, they need to do extra work.

The index of an entity can be used as a direct replacement in most situations.

IDs are required if you use PooledEngine and want to store a reference to an entity that might be destroyed.

That's probably doing something pretty wrong. If you really want that, you can use a wrapper class for handling entity references.

For what purpose ? Seems to me like there is only overhead, but no benefit.

Because then there is no need to assign a magic value to denote the absence of an ID. The overhead caused by adding an extra component type is redundant.

Right now, Ashley is bad memory-wise. It stores the components of an entity in both an Array and sparse Bag

might be true, but that's no excuse to worsen it.

But this does not matter too much, as today's computers and VMs are capable of handling it.

on a pc maybe.. mobile devices not so much

The index of an entity can be used as a direct replacement in most situations.

Can't think of any where it would.

That's probably doing something pretty wrong. If you really want that, you can use a wrapper class for handling entity references.

So adding lots of entitylisteners with if(entity == this.entity) entity = null; is better than storing an id ?
I'd rather check if the entity still exists when I need it than getting constant updates about every entity creation and deletion.

Because then there is no need to assign a magic value to denote the absence of an ID. The overhead caused by adding an extra component type is redundant.

If you just want the magic value gone, why not make it a reference? Tho I don't think it's much "magic", since -1 or 0 is quite common for this case, especially if the values are always positive.

might be true, but that's no excuse to worsen it.

Assuming you're talking about component storage: it would improve it.

on a pc maybe.. mobile devices not so much

Yeah, but I don't think you will use thousands of entities in a mobile game. If so, you're better off not using Ashley at all.

Can't think of any where it would.

Tell me where it wouldn't.

So adding lots of entitylisteners with if(entity == this.entity) entity = null; is better than storing an id ?
I'd rather check if the entity still exists when I need it than getting constant updates about every entity creation and deletion.

No; like I said, you can introduce an EntityReference class, and something like EntityReferenceManager, that maintains a sparse Bag weak hash map of entity index->EntityReference, and invalidates references as entities are removed. One possibility that I've thought of is to auto-assign negative IDs to created entities, and require that manually assigned IDs be positive. I do, however, think of this as quite bad.

If you just want the magic value gone, why not make it a reference? Tho I don't think it's much "magic", since -1 or 0 is quite common for this case, especially if the values are always positive.

If we don't need a magic value, then it's possible to permit any number as ID. Storing IDs in a component also allows processing entities with an ID separately, which could be useful for some system implementations.

I don't think it's a good idea to keep unique identifiers; they can easily be stored in components and managed by systems. The idea of an index might be useful for future optimizations, but it's probably not needed yet. I suggest that we remove support for unique identifiers, and let users implement their own components.

For my game, I used a UUIDComponent class, which uses 2 longs to represent a 128 bit UUID. I send things to and from the server as an array of components, and then check UUID's to figure out how to handle the packet.

Fixed by #182.