Zay-es-net: ComponentFilter not working?

qxCsXO1 · July 16, 2015, 11:07am

Hi,

I am developing a multiplayer game where players should not be able to see entities of other players.
(At least for now, later they should also be able to see all nearby entities).

To implement this I tag every player related entity with this component:

public class OwnedByPlayer implements EntityComponent, PersistentComponent {
	private EntityId playerID;

	public OwnedByPlayer() {

	}

	public OwnedByPlayer(EntityId playerID) {
		this.playerID = playerID;
	}

	public EntityId getPlayerID() {
		return playerID;
	}

	@Override
	public String toString() {
		return "OwnedByPlayer[playerID=" + playerID.getId() + "]";
	}
}

At the client side I use a FieldFilter to only get a players own entities, e.g.

ed.getEntities(FieldFilter.create(OwnedByPlayer.class, "playerID", accountID), Position.class);

However the client receives component updates which are tagged with a different players ID.
Any idea why this is happening?

pspeed · July 16, 2015, 2:37pm

You are filtering on a component that you are not retrieving. You need to also get the OwnedByPlayer component, ie: add the class after Position.class.

qxCsXO1 · July 16, 2015, 2:44pm

Above was just an arbitrary example, I am including OwnedByPlayer in all queries.

nehon · July 16, 2015, 3:05pm

Please…post your actuall code, so the real issue can be found…

normen · July 16, 2015, 3:12pm

Also note that using inheritance for components is more or less asking for problems down the line. Its a good hint that you’re doing Entity Systems wrong somehow.

qxCsXO1 · July 16, 2015, 3:15pm

Where am I using inheritance?

normen · July 16, 2015, 3:17pm

public class OwnedByPlayer implements EntityComponent, PersistentComponent

qxCsXO1 · July 16, 2015, 3:23pm

Just to be clear: I am using zay-es.
If I am not completly wrong components have to implement the (empty) interface EntityComponent and additionally the interface PersistentComponent if i am using the SQL backend.

pspeed · July 16, 2015, 3:44pm

Well, your arbitrary example wouldn’t work but a proper example will work. I use it all the time.

I can only actually comment on code shown… I can’t guess about code I can’t see.

pspeed · July 16, 2015, 3:44pm

Those are Zay-es defined interfaces marking it as a component and a persistent component.

normen · July 16, 2015, 3:45pm

Ah, didn’t know about PersistentComponent

qxCsXO1 · July 16, 2015, 4:10pm

I have created a minimal test case to show the wrong behaviour
The server:

import java.io.IOException;

import com.jme3.app.SimpleApplication;
import com.jme3.math.Vector3f;
import com.jme3.network.Network;
import com.jme3.network.Server;
import com.jme3.network.serializing.Serializer;
import com.jme3.network.serializing.serializers.FieldSerializer;
import com.jme3.system.JmeContext.Type;
import com.simsilica.es.Entity;
import com.simsilica.es.EntityData;
import com.simsilica.es.EntityId;
import com.simsilica.es.EntitySet;
import com.simsilica.es.Name;
import com.simsilica.es.base.DefaultEntityData;
import com.simsilica.es.net.EntitySerializers;
import com.simsilica.es.server.EntityDataHostService;

public class TestServer extends SimpleApplication {
	public static final int PORT = 12345;

	private Server host;
	private EntityDataHostService edHost;
	private EntityData ed = new DefaultEntityData();
	private EntitySet set;

	static {
		EntitySerializers.initialize();

		Serializer fieldSerializer = new FieldSerializer();
		Serializer.registerClass(OwnedByPlayer.class, fieldSerializer);
		Serializer.registerClass(Position.class, fieldSerializer);
	}

	@Override
	public void simpleInitApp() {
		try {
			host = Network.createServer("Test", 1, PORT, PORT);
			host.addChannel(PORT + 1);

			edHost = new EntityDataHostService(host, 0, ed);

			host.start();
			initData();
		} catch (IOException e) {
			e.printStackTrace();
		}
	}

	private void initData() {
		EntityId player1 = ed.createEntity();
		EntityId player2 = ed.createEntity();

		System.out.println("player1: " + player1);
		System.out.println("player2: " + player2);

		ed.setComponent(player1, new Name("Player 1"));
		ed.setComponent(player2, new Name("Player 2"));

		EntityId unit1 = ed.createEntity();
		EntityId unit2 = ed.createEntity();

		System.out.println("unit1: " + unit1);
		System.out.println("unit2: " + unit2);

		ed.setComponents(unit1, new Position(new Vector3f(0, 0, 0)),
				new OwnedByPlayer(player1));
		ed.setComponents(unit2, new Position(new Vector3f(100, 0, 0)),
				new OwnedByPlayer(player2));

		set = ed.getEntities(Position.class);
	}

	@Override
	public void update() {
		for (Entity e : set) {
			Position pos = e.get(Position.class);
			e.set(new Position(pos.getPosition().add(0, 1, 2)));
		}

		edHost.sendUpdates();
	}

	public static void main(String[] args) {
		TestServer server = new TestServer();
		server.start(Type.Headless);
	}
}

The client:

import java.io.IOException;

import com.jme3.app.SimpleApplication;
import com.jme3.network.Client;
import com.jme3.network.ClientStateListener;
import com.jme3.network.Message;
import com.jme3.network.MessageListener;
import com.jme3.network.Network;
import com.jme3.network.serializing.Serializer;
import com.jme3.network.serializing.serializers.FieldSerializer;
import com.jme3.system.JmeContext.Type;
import com.simsilica.es.Entity;
import com.simsilica.es.EntityId;
import com.simsilica.es.EntitySet;
import com.simsilica.es.client.RemoteEntityData;
import com.simsilica.es.filter.FieldFilter;
import com.simsilica.es.net.ComponentChangeMessage;
import com.simsilica.es.net.EntitySerializers;

public class TestClient extends SimpleApplication {
	private Client client;
	private RemoteEntityData ed;
	private EntitySet set;

	private static final int PLAYER_ID = 0;

	static {
		EntitySerializers.initialize();

		Serializer fieldSerializer = new FieldSerializer();
		Serializer.registerClass(OwnedByPlayer.class, fieldSerializer);
		Serializer.registerClass(Position.class, fieldSerializer);
	}

	@Override
	public void simpleInitApp() {
		try {
			client = Network.connectToServer("Test", 1, "localhost",
					TestServer.PORT, TestServer.PORT);
			client.addMessageListener(new MessageListener<Client>() {
				@Override
				public void messageReceived(Client source, Message m) {
					if (m instanceof ComponentChangeMessage) {

					}
					System.out.println("Received: " + m.toString());

				}
			});
			ed = new RemoteEntityData(client, 0);
			client.addClientStateListener(new ClientStateListener() {

				@Override
				public void clientDisconnected(Client c, DisconnectInfo info) {

				}

				@Override
				public void clientConnected(Client c) {
					set = ed.getEntities(FieldFilter.create(
							OwnedByPlayer.class, "playerID", new EntityId(
									PLAYER_ID)), OwnedByPlayer.class,
							Position.class);

				}
			});
			client.start();

		} catch (IOException e) {
			e.printStackTrace();
		}
	}

	@Override
	public void update() {
		if (set != null) {
			if (set.applyChanges()) {
				for (Entity e : set.getChangedEntities()) {
					OwnedByPlayer own = e.get(OwnedByPlayer.class);
					System.out.println(own.toString());
				}
			}
		}
	}

	public static void main(String[] args) {
		TestClient client = new TestClient();
		client.start(Type.Headless);
	}
}

Output of the server:
player1: EntityId[0]
player2: EntityId[1]
unit1: EntityId[2]
unit2: EntityId[3]

Output of the client:

Received: ComponentChangeMessage[[EntityChange[EntityId[3], Position[position=(100.0, 237.0, 474.0)], class Position], EntityChange[EntityId[2], Position[position=(0.0, 237.0, 474.0)], class Position]]]
Received: ReturnComponentsMessage[72, EntityId[3], [OwnedByPlayer[playerID=1]]]
OwnedByPlayer[playerID=0]

So you can see the client gets the updates for both units (EntityId 2 and 3), where he should only get one for ID 2.
In the EntitySet only the correct Entity arrivies (OwnedByPlayer[playerID=0]).

pspeed · July 16, 2015, 4:18pm

Oh, wait… so you are not actually seeing bad behavior you are just concerned about the extra messages underneath?

It’s difficult to decide on a component-by-component level what changes should or should not be forwarded to a client, I think. I sort of remember some 3-4 paragraph comments in the source files to this affect but I can’t look at the moment. I’d have to look in more detail to be sure.

Also, to be sure, are you building Zay-ES from source or using the real releases. I know the published releases are behind in a few respects so I want to be sure.

qxCsXO1 · July 16, 2015, 4:24pm

I am using the latest source from google code (rev. 1607).
On the application level everything works as intended, but there a 2 major problems with your approach:

Performance: sending way more information than needed over the network.
Security: players receive information they should not see, very very bad in a competitive game. Just imagine you could see every enemy unit in StarCraft 2.

pspeed · July 16, 2015, 4:29pm

Yeah, I understand it’s less than ideal but I think there is a reason for it is all I’m saying.

I will look into it soon but in the mean time if you look at the source you might see an explanation. Perhaps it’s just a bug though and you will spot that too.

qxCsXO1 · July 16, 2015, 5:03pm

Fixed it for you:

Index: zay-es/src/com/simsilica/es/base/DefaultEntitySet.java
===================================================================
--- zay-es/src/com/simsilica/es/base/DefaultEntitySet.java      (Revision 1607)
+++ zay-es/src/com/simsilica/es/base/DefaultEntitySet.java      (Arbeitskopie)
@@ -716,8 +716,7 @@
             if( updates != null ) {
                 if( comp == null 
                     || filters == null 
-                    || filters[index] == null 
-                    || filters[index].evaluate(comp) ) {
+                    || mainFilter.evaluate(e.get(mainFilter.getComponentType())) ) {
                     
                     updates.add(change);
                 }

pspeed · July 16, 2015, 7:25pm

But I think the problem is then that if the condition changes the entity won’t get removed from the set.

pspeed · December 27, 2015, 9:35am

So, I spent the last few days setting up a simple test so that I could look into this issue in more detail because sending filtered data over the wire shouldn’t be happening.

And well, as it turns out it isn’t. (And also removes aren’t getting sent either but that’s another issue that I will fix a different way.)

The issue I have with your code snippet is that it shouldn’t be necessary unless you have a bad filter. For entity sets, you can only filter on one component type at a time by design.

So basically:
filters[index].evaluate(comp)
and
mainFilter.evaluate(e.get(mainFilter.getComponentType()))

…should be identical in all valid cases as filters[index] should already be mainFilter.

If you are still following this thread at all, I’d be curious to know more about your use case.

On the plus side, even though my test shows no problem I can at least sleep better at night as this we bothering me. Now I know for sure in great tracing detail what’s happening. I also found a different bug completely which will be nice to fix.

Edit: actually I see now why the condition is not the same. I have to think about it a second. Still, I’m seeing no additional data sent for filtered items so I’m very curious why this comes up in your case.

pspeed · December 27, 2015, 9:38am

Ah. I get it now. You are filtering on a different component than your changing value.

I will look in more detail as this use-case never comes up for me normally. I filter on the value that’s changing.

pspeed · December 27, 2015, 10:10am

So, because I enjoy talking to myself as I work through problems…

I really really wanted your solution to be the one because it’s very simple. Unfortunately, some thought experiments show that it can lead to incorrect behavior in certain cases.

For example, an entity set that is watching Name, OwnedBy, and Position. If the OwnedBy component excludes a particular entity but then the code changes both Name and OwnedBy at the same time, it could be the case that the Name change never gets sent to the client because the OwnedBy change hadn’t been processed yet.

Your addition to the if statement would exclude the Name change because OwnedBy still doesn’t match yet. If Name changes rarely then the client will never see the proper value on a subsequent frame.

So the real solution is going to be more complicated. I may have to post process the updates set though I’d really like to avoid it.