Advertisement

Kryonet server/client code review

Started by April 17, 2018 09:43 AM
5 comments, last by hplus0603 6 years, 7 months ago

I have a MongoDB db on a server operated by Kryonet. Obviously I need to be able to query the database from the client for adding, removing, requesting all kind of assets. Now I have written the code to request, add and remove fighters on the Kryonet network I am wondering if there is a better way to do this. I feel it's a bit repetitive, especially when I need to implement this for all other assets the player can own and other players assets when needed. The way I am currently approaching this is the same as my chat/lobby system which works great but I was wondering if anyone could see improvement on my code or a complete different way that is much more scalable perhaps.

 


public class ClientAssets {
	public static final int FIGHTER_REQUEST = 1;
	public static final int FIGHTER_RESPONSE = 2;
	public static final int FIGHTER_ADD = 3;
	public static final int FIGHTER_REMOVE = 4;

	public static void Register(EndPoint endPoint) {
		Kryo kryo = endPoint.getKryo();
		
		kryo.register(FighterRequest.class);
		kryo.register(FighterResponse.class);
		kryo.register(FighterAdd.class);
		kryo.register(FighterRemove.class);
	}

	static public abstract class AssetPacket {
		public int packetId;

		public AssetPacket() {
		}
	}

	/**
	 * Packet to request all owned fighters
	 */
	public static class FighterRequest extends AssetPacket {
		public ObjectId playerId;
		public FighterRequest(ObjectId playerId) {
			packetId = FIGHTER_REQUEST;
			this.playerId = playerId;
		}

		public FighterRequest() {
		}
	}

	/**
	 * Receiving fighter data from server
	 */
	public static class FighterResponse extends AssetPacket {
		public Fighter fighter;
		public boolean add; // Add or remove
		public FighterResponse(Fighter fighter, boolean add) {
			packetId = FIGHTER_RESPONSE;
			this.fighter = fighter;
			this.add = add;
		}

		public FighterResponse() {
		}
	}

	/**
	 * Adds a fighter to player assets
	 */
	public static class FighterAdd extends AssetPacket {
		public ObjectId fighterTemplateID;
		public FighterAdd(ObjectId fighterTemplateID) {
			packetId = FIGHTER_ADD;
			this.fighterTemplateID = fighterTemplateID;
		}

		public FighterAdd() {
		}
	}

	/**
	 * Removes fighter from assets.
	 */
	public static class FighterRemove extends AssetPacket {
		public ObjectId fighterId;

		public FighterRemove(ObjectId fighterId) {
			packetId = FIGHTER_REMOVE;
			this.fighterId = fighterId;
		}

		public FighterRemove() {
		}
	}
}

To elaborate a bit more, this code will communicate between client and server. When receiving a request on the server it will lookup the request in the database. The client will store it for displaying the assets. A specific thing I am unsure about is the FighterResponse.add boolean. I need to be able to remove and add fighters, I guess I am better off with a FighterAddResponse and a FighterRemove response so I will send one boolean less each time this packet is send. But this will create even more repetitive code.

Obviously I need to be able to query the database from the client for adding, removing, requesting all kind of assets. 

Who is the client? The end user / player? You don't want your end user to ever talk directly to your database.

I don't know KryoNet, though -- does it allow you to write code on the server side to validate user requests?

When it comes to declaring structures for messaging, yeah, you have to declare those structures, and often you have to add some boilerplate for each structure. Each network API will have some set of requirements and some set of helpers that give you more or less help with this. As far as what I see above, that doesn't look particularly verbose compared to other systems.

Finally, we can't read your mind. What is a "Fighter?" Is it a player in a fighting game? Is it a vehicle in a flight simulator game? Is it a resource in a galactic strategy game? Specific advice may vary based on how the data are supposed to be used.

 

enum Bool { True, False, FileNotFound };
Advertisement

The client is the end user and the code I wrote is for client/server communication. The server receives a packet and can handle the request. What I meant with the line I wrote is that the client does need to ask for data in the database but trough a client/server connection.

 

The classes I wrote will be registered on both client and server so it can be serialized/deserialized by Kryo and send over. On both the client and server I would setup a listener for this packets. Whenever something arrives I check what kind of packet it is and then check the packet ID to know the exact request or response and handle that. The following will happen:

  • Client logs in and needs his fighters to be displayed. He sends a FighterRequest packet to the server.
  • Server listens for these packets, receives the packet and will query the database for all clients fighters by looking up the playerId.
  • For each fighter the server sends back a FighterResponse packet.
  • The client listens for this packets, receives the packets and handles it so the user can see his fighters.

Now if I want my client to receive a dialog when he buys or wins a new fighter I need another such packet class so the client can handle it correctly. And thus for every instance I need slightly different functionality I need such a packet class. They all feel rather similar though, I guess whenever the player requests data the type of data to request can be represented by a simple int or enum. 

Fighter, item, login details, etc does not matter.  They are just data bags and atm I am just fetching them for the client to display them, later they can fight in a turn based fashion but that would still be the same data bag.

Why does the client need to request the data at all?

Why wouldn't the server just send the data that the client needs, when the client logs in?

 

enum Bool { True, False, FileNotFound };
On 4/18/2018 at 7:10 PM, hplus0603 said:

Why does the client need to request the data at all?

Why wouldn't the server just send the data that the client needs, when the client logs in?

 

The player can add/remove/alter the data like leveling up and add attributes and stats or change the name. I wanted to avoid writing a packet for each change. I would just send the new data, verify that on the server and send it back. The player also needs access to other clients data.

But I guess it could work, I am sending all the initial data on login already. So if a player levels up a Fighter he can send something allong the lines of



	public static class updateFighter extends FighterPacket {
		public ObjectId fighterId;
		public Attributes attributes; // attributes to improve;
		public int skillId; // Skill to add, skills not implemented yet.

		public updateFighter() {
		}
	}	

 

He then waits for confirmation of the server. If the server response is negative the data reverts to the old state otherwise the server does the same and the data should be in sync.

 

Quote

if a player levels up a Fighter he can send something allong the lines of

If you want players to be able to arbitrarily increment their attributes by sending hand-crafted network packets, that seems like a good idea!

Assuming you have "level points" that can be spent, the way I prefer to think of the transaction would be that you "buy" the improvements by "spending" level points, and thus the message would be "SpendLevelPoints" and the message would contain how many level points are expected to be spent, and what is being bought.

Maybe you then have many things that can be spent, and many things which can be bought, and you then end up writing a typical double-entry accounting system, where the server verifies that you're not trying to spend "negative" level points, or spend more level points than you have in the level point account. How to write RPC systems for double-entry accounting is well covered in literature, because every business does this :-) (Typically, you have a chart-of-accounts which tells you what each thing means, and then send a transaction with a number of rows, where the sum of all rows evens out to zero.)

For the more general question of: "How do I avoid writing stubs for each of my RPC affordances?"
The answer is "you don't, really, unless you use an IDL that generates the code for you." (IDL == Interface Definition Language.)
It looks like Kryonet already uses reflection to do most of the IDL work, so the code you have to write for each message isn't dramatically larger than what you'd have to write in an IDL, so it looks like you're not in a bad place already.

 

enum Bool { True, False, FileNotFound };

This topic is closed to new replies.

Advertisement