patternjavaModerate
MMO Game Server
Viewed 0 times
servermmogame
Problem
I've been building an MMO in Java for a game that will have clients built with libGDX. I have already built clients for the browser, desktop, iOS, and Android. To accommodate multiple platforms, websockets are used, and all messages sent back and forth are in the form of strings.
The purpose of the scheduled
I have removed some of the code (mostly password handling and the saving and loading of game and player data) so that it is more concise.
I would like to hear about the readability and overall organization of the code. This is the largest project that I have written in Java so far, and I believe I am still not following all the best practices or taking advantage of all the language features. If you see any architectural failings, those would be great to hear about too.
If you would like to see any other methods, please let me know. The main function of the
```
public class Server {
private final ServerSockets websocketServer;
private final int maxIpConnectionsThreshold = 10;
private ArrayList clients = new ArrayList();
private HashMap recentIpAddresses = new HashMap();
private MainGame game;
public boolean isLoadingOrSaving = false;
private BZLogger messagesRecievedLogger = new BZLogger("messagesReceived", "messagesReceived.log", false);
private BZLogger messagesSentLogger = new BZLogger("messagesSent", "messagesSent.log", false);
private BZLogger systemMessagesLogger = new BZLogger("systemMessages", "systemMessages.log", true);
/**
* When the backupCount reaches X, the player and wo
The purpose of the scheduled
refreshAllClients() method is to try to update all clients at a 100ms delay. I have heard that this is a good approach for an MMO, but I am not totally sure about this.I have removed some of the code (mostly password handling and the saving and loading of game and player data) so that it is more concise.
I would like to hear about the readability and overall organization of the code. This is the largest project that I have written in Java so far, and I believe I am still not following all the best practices or taking advantage of all the language features. If you see any architectural failings, those would be great to hear about too.
If you would like to see any other methods, please let me know. The main function of the
BZLogger class is to log to a file while optionally allowing to log to the console. The websocketServer class has listeners for things like onMessage or onOpen, but everything is forwarded to the Server. I'm using the java_websocket library.```
public class Server {
private final ServerSockets websocketServer;
private final int maxIpConnectionsThreshold = 10;
private ArrayList clients = new ArrayList();
private HashMap recentIpAddresses = new HashMap();
private MainGame game;
public boolean isLoadingOrSaving = false;
private BZLogger messagesRecievedLogger = new BZLogger("messagesReceived", "messagesReceived.log", false);
private BZLogger messagesSentLogger = new BZLogger("messagesSent", "messagesSent.log", false);
private BZLogger systemMessagesLogger = new BZLogger("systemMessages", "systemMessages.log", true);
/**
* When the backupCount reaches X, the player and wo
Solution
Some quick things that I can see :
You're returning the implementation, you should almost always flavor the interface, in this case
Auto-generated code should be changed almost immediately. Printing to the stacktrace is most of the time not how you want to manage things. If you really don't know how to manage this, log the error with the stacktrace and be done with it. A log has more chance to be look at and be saved somewhere than the console output is.
....
This piece of code is rather not that readable. I would first maybe create String version of
public ArrayList getClients() {
return this.clients;
}You're returning the implementation, you should almost always flavor the interface, in this case
List. Hiding implementation is a good thing. try {
this.loadWorld();
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}Auto-generated code should be changed almost immediately. Printing to the stacktrace is most of the time not how you want to manage things. If you really don't know how to manage this, log the error with the stacktrace and be done with it. A log has more chance to be look at and be saved somewhere than the console output is.
String[] messageFrags = message.split(delimiter);
if (messageFrags[0].equals(String.valueOf(MessageType.PLAYER_REGISTRATION_MESSAGE.id())))....
This piece of code is rather not that readable. I would first maybe create String version of
MessageType.PLAYER_REGISTRATION_MESSAGE.id() since it would remove a lot of not so useful transformation. For the if itself, you can use a switch for a String maybe it would be a solution.Code Snippets
public ArrayList<Client> getClients() {
return this.clients;
}try {
this.loadWorld();
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}String[] messageFrags = message.split(delimiter);
if (messageFrags[0].equals(String.valueOf(MessageType.PLAYER_REGISTRATION_MESSAGE.id())))Context
StackExchange Code Review Q#89964, answer score: 12
Revisions (0)
No revisions yet.