Ok so you want constructive critic ?
Here we go.
So first off just follow one rule: One class/enum per file. Filename = Classname. Would make things more clear.
Document your things. There are nearly no comments at all. When there is stuff commented it's just stupid like this:
Code:
NewConnected(); // Check If New Connected
You should make the names of the methods expressive. Good code explains itself.
Stay to one naming convention.
Use given functions from dictionarys.
Code:
if (Core.Config.Users.ContainsKey(userId))
{
name = Core.Config.Users[userId].GetUser().Name;
userConfig = Core.Config.Users[userId].GetUser().Settings;
mapId = Core.Config.Users[userId].GetUser().MapId;
shipId = Core.Config.Users[userId].GetUser().ShipId;
factionId = Core.Config.Users[userId].GetUser().FactionId;
x = Core.Config.Users[userId].GetUser().X;
xz = Core.Config.Users[userId].GetUser().XZ;
y = Core.Config.Users[userId].GetUser().Y;
yz = Core.Config.Users[userId].GetUser().YZ;
toX = Core.Config.Users[userId].GetUser().ToX;
toY = Core.Config.Users[userId].GetUser().ToY;
//... much more of such shit
Can be made to this
Code:
Conn value;
if (Core.Config.Users.TryGetValue(userId, out value))
{
name = value.GetUser().Name;
userConfig = value.GetUser().Settings;
mapId = value.GetUser().MapId;
shipId = value.GetUser().ShipId;
factionId = value.GetUser().FactionId;
x = value.GetUser().X;
xz = value.GetUser().XZ;
y = value.GetUser().Y;
yz = value.GetUser().YZ;
toX = value.GetUser().ToX;
toY = value.GetUser().ToY;
That version will be faster because the old one called too many functions.
Write wrappers around the raw strings used in packets. Will be cleaner.
I could have gone into depth with every single of that suggestions but I will not because I have better things to do. Just remember those things when programming.
I just wanted to say that I think it is a shame that this is the official emulator for such an great cms.
In-Orbit Official Emulator
