Edit:
Looks like I can return the favour
Compiling the network branch I got an interesting warning, "warning C4804: '>=' : unsafe use of type 'bool' in operation"
server_interface.cpp(59) :
if(!getPeers().size() >= GameConstants::maxPlayers) {
'not' has a higher precedence than the other logical operators, you need another set of parenthesis
good catch! =)
... I ended up going with 'cornerHolder', it could have just been 'corner' the whole time of course, and then set to -1,-1 when not of interest anymore. But I obviously felt compelled to use a pointer... old habits die hard
Regarding pointers, I have no problem with this where performance is at issue. I prefer references to pointers, but this is only feasible where you will continue to reference the same object throughout a function call or code block. Otherwise, use a pointer. Copying a pointer is always 1 or 2 instructions. Copying an object (even small) is rarely that few. However, if the function is called a lot, you can make cornerHolder "static const" so it's only allocated once (and const so you don't accidentally change it
.
But then I will quote two oft stated statements (that repetition was redundant huh?
"First make it right, then make it fast" and "85% of your CPU is spent in 5% of your code" (or some such). So only worry about tweaking the high-use stuff and if the rest is a bit flabby, that's probably OK. It's often a trade-off between beauty of design and performance. Optimizations can be ugly and if we take a few more bytes & CPU cycles for code that doesn't execute a lot, then the flabbiness is OK -- especially if the trade-off is a cleaner design, which results in higher maintainability (i.e., it's easier to understand and modify and harder to make an error when it is modified).
One prime example (that I still tend to get fussy about) is exception handlers: they very seldom execute and can therefore be fat and slow. I know I tend to scoff at operator+(std::string, std::string) because it's flabby, but it really doesn't matter in exception handlers and I'll try to chill out on that. The only thing about using string's operator+ is that we end up having to call our own custom conversion functions and I would rather use iostreams for that since it handles it so well. The draw back of that is that it takes 3 lines of code where operator+ can be hacked together in one.
EDIT: Another area of note is that my network code makes use of a lot of virtual function calls. I don't care here because network data is only handled a few times a second, and it's not time critical (as long as it doesn't bloat in size unnecessarily). The ObjectPrinter stuff is something that I may encapsulate in some pre-processor #if/#endif however, and leave them empty in-line implementations (i.e., "compile them out") if some value is set when compiled because that does add measurable bloat. On the other hand, it will greatly aid in troubleshooting network problems in the initial stages of this new network code, so I don't plan on releasing the 1st time with that compiled out.
Sounds good, a more functional GameSettings object would also make me feel better about it actually being a class... I remember reading about the 'new game settings model' a while back, I'll have to have a good look at the network branch some time...
Well, I just hope I don't revisit my new GameSettings and decide that it's crap! lol! I remember it not feeling very flexible when I was done, but I haven't looked at it closely in the last several months. One of the paradigms that I remember it following correctly was de/serialization to/from XML -- that makes it work both accross the network and for saved games. At currently, there is still far too much data being sent as zlib-compressed XML, but the idea here is that it's easy to troubleshoot and debug and that the fat part (unit updates & the like) can later be implemented using a binary protocol that's much tighter -- after the major flaws are resolved.
I need to regenerate the Doxygen docs (with the cute UML diagrams), but in essence, the new GameSettings consists of (in summary):
class GameSettings : public XmlWritable, Uncopyable {
public:
typedef map<int, shared_ptr<Player> > PlayerMap;
class Team : public IdNamePair, Uncopyable { ... }
typedef vector<shared_ptr<Team> > Teams;
class Faction : public IdNamePair, Uncopyable { ... }
typedef vector<shared_ptr<GameSettings::Faction> > Factions;
private:
PlayerMap players;
Teams teams;
Factions factions;
}
players is a map of all players, indexed by their player ID (I think the server is always zero or some such) including computer players. Game::Player is an abstract base class with Game::HumanPlayer and Game::AiPlayer subclassing it and the human player "has a" Game::Net::NetworkInfo object (unless you're playing a local game). Then the GameSettings has a list of
teams (Game::GameSettings::Team) and
factions (Game::GameSettings::Faction). So a game must have at least one team, although less than two teams isn't very useful out side of scripting some cut scene in a campaign or some such. Then each faction belongs to exactly one team. However, a player can control zero or more factions (zero if they are an observer/spectator) and each faction must be controlled by at least one player. This allows a game setup where people can share control of their faction with others, have two human players controlling the same faction or even share control with an AI.
While I think this design encompasses a lot of possibilities, I don't think the implementation is especially flexible, and that may be OK. What I think is needed to take it to the next level, however, is for GameSettings (along with it's teams, factions, players (both Human and AI)) to contain arbitrary name/value pairs, vectors and maps based upon a given mod -- but I'm not worrying about that for now.
In short, I think it's a good step forward, but will also have to be revisited a number of times.
EDIT: OK, sorry for the long post. I also wanted to mention that shared_ptr is a new thing in my repertoire and I've learned that it's quite the valuable pattern. It essentially allows you a lot of flexibility on when an object is allocated and who will de-allocate it and solved a lot of the problems I had as a result of not wanting to implement a copy constructor for GameSettings (since I thought it was wasteful -- maybe I was being too picky
.