Multiple inheritance

I’m not sure whether or not to be scared. I’ve been bleeding away dev time on the instancing work due to the lack of a standard definition of “The Player” between hosts processes. It was also a fairly large part of what kept me from delivering dynamic missions.

With no basal definition of a player within the system, who’s authoritative for any given item can be a nightmare.

So I had to introduce a new fundamental… class wwii::Player.Fortunately for me, I’ve converted a lot of the host code to at least rudimentary C++. I’ve gradually introduced a slew of standardized member functions into structures that otherwise share very little in common — for instance, the playerID may be present in four different substructures and called uPlayerID in the chat host’s Player class, while it’s only in the “AuthDispatch” member of the cell host and on the map host its in 6 different places… But there was a member function in each, playerID(), that [generally] got the right one, or one that was probably accurate.

That made phase 1 pretty easy – introducing a new base class to each of the hosts own “Player” class. That opens up countless doors to code simplification, lots of bits of code don’t need to be specific to a specific server process. But … that’s for another time.

Perhaps my biggest obstacle was that most of our classes initialize themselves with a memset. C++ constructors are crap: maintaining constructors for a class with a hundred members is a pain, and then you can’t initialize arrays or char arrays anyway. A good old memset is far more to the point and efficient, just fatal if you’re using classes with vtables.

So … wwii::Player is a non-virtual base parent for now… ick. Oh, and C++ doesn’t provide a way to say “thou shalt not override this function”. Gulp.

Of course, there was an obstacle to my doing this before: the spawn mechanism. Spawning is fundamentally a change of authoritative server process for your session. You start out on the map host, it gets a vehicle sent to the cell host and then it tells the network/session API “transfer this player to that cell host”.

This is the same process by which you get transfered from the auth host to the map host in the first place, it’s the same mechanism that causes you to automatically open a connection to the chat host as you spawn in.

Just before the map host spits you out, though, it stuffs a little extra data into a blank space at the end of the redirect message. This is a shared data structure between the two hosts: it describes who you are, your permissions, the context you are spawning in and a bunch of blanks for the host to fill out to summarize your in-sortie activities.

Thus, when you are done and despawn, the cell host simply reuses the same method and structure to send you back to the map.

If the map host has crashed and restarted, this excess of data allows it to reconstruct its concept of who you are and drop you back into the system good as new (almost).

Except, between the two hosts, this duplicated information has slowly taken authoritativeness over what else the particular server knows about you. Fundamentally, it was begging for a common Player description…

In the absence of one, and because the cell host has to have been told about you preparing to arrive before the redirect occurs, the cell hosts have their own host-specific instance of a Player definition, which includes space for the mission packet when you log in; data from the mission packet and the Player definition are swapped like so many bodily fluids during the connection establishing process.

Further complicating this (no, really, further!) is the fact that the MissionPacket is the context in which your vehicle is spawning. When the guy who owns the vehicle joins it, his MissionPacket gets copied into the vehicle – player specific data and all. The separate copies exist to cope with multi-crewing. Unfortunately, theMissionPacket was an all in one and, obviously, confusion ensued.

Which is where we come to multiple-inheritance. In introducing the standard wwii::Player definition, I’d gone after data duplication with prejudice, which mean’t I’d stripped it out of the MissionPacket. For the purposes of network transit and minimizing the amount of code that would get changed, I wanted to recreate the network view of an all-in-one concept.

So I created MissionPacketXfer which inherits both wwii::Player and MissionPacket. That gives it all the members and member functions of both classes but it also allows me to trivially separate the data out again and the cell-host specific Player class derived from wwii::Player happily allows me to create a new instance via a fairly clean and simple constructor. (This is a transliteration that I’m not going to error check)

class Host::Player
{
Player(const MissionPacketXfer* xfer)
: wwii::Player(*static_cast<const wwii::Player*>(xfer)
, m_missionPacket(*static_cast<const MissionPacket*>(xfer)

} ;

// Receiving a MissionPacketXfer
Host::Player* incomingPlayer = new Host::Player(incomingXfer) ;

Best of all, the MissionPacket, stripped of its Player specific details no-longer clutters up the vehicle definition which has probably already fixed a couple of daft multi-crew bugs. In fact, the next logical step – for later – is to remove the MissionPacket from the individual players and make it vehicle specific or – better still – make it a reference counting smart pointer.

I started writing this post earlier today, when it seemed I was elbow deep in host-code guts. The theory was “this approach should get the code delivered in the shortest possible order without closing the door to easy refactoring to what I’d consider Best Practices in the future”. I wasn’t really quite nervous about that “shortest” part – for the last 2-3 weeks I’ve been locked in mortal combat with legacy and “surprise” server code…

But the approach and my ongoing methodology seem to have paid off. The first attempt was a fail, but that’s down to a stupid void* pointer allowing me to send the address of the map hosts copy of the MissionPacketXfer rather than the data in it. With that fixed in a matter of minutes, the host was up and (so far) working fine.

The encapsulations I’ve imposed upon myself mean that actually relatively little changed – much of what I had to do was do things like replace player->missionPacket().playerID() with player->playerID() und so weiter.

Of course, it may explode in horrible ways – multicrew may be totally broken, but that was stuff I was expecting to be testing this weekend. The fact that stuff is standardized means that I can probably take an hour or two tomorrow writing some unit tests and get some really good test coverage.

After that, tomorrow afternoon, I get to dive back into the instancing code and actually turn it on. There’s a bunch of feature I’ve worked out with Ramp in terms of deciding how we expose the list of training instances, control their lifespan and their activation etc, but again the encapsulation I’ve used in this final iteration lends itself beautifully to integrating that stuff cleanly between client and host.

Merging the client and host codebases (when we moved to Subversion) has been a beautiful part of that, and the bits of client-build cleanup I’ve been doing (which have probably given Rafter and Gophur gray hairs because it seems so unrelated) have saved me aeons of time.

It’s a much nicer working environment, too; I really hope to get some time to set up some build procedures with the shiny new FinalBuilder license the guys at VSoft were kind enough to give me.

Most of all, though, it’s nice to see good coding practices win out this time. Now, if I can just convince the C++ people that we need to be able to do enum prototypes in member functions so that we don’t have to include the enum definitions every time we want to have a strongly typed enumeration list but very few things will use the member function

class Foo
{

public:
void printError(enum LocMessageID locMessage, const char* argument) ;

} ;

That’d allow me to massively clean up the #include lists and make our build environment much, much healthier. That and a better way to declare a struct/class as needing to be created initialized to zeros without the headache of having to write your own allocator or write ridiculously long winded constructors every time.

class Foo
{
public:
Foo()
: m_member1(0), m_member2(0), /* member3 is an array, can’t initialize it */
, m_member5(TYPED_ENUM_OF_VALUE_0)
, m_member8(NULL)

, m_member102
{
memset(m_member3, 0, sizeof(m_member3)) ;
m_member4[0] = 0 ; // Character array
memset(m_member6, 0, sizeof(m_member6)) ;

}
Foo(unsigned int valueForMember1)
: m_member1(valueForMember1), m_member2(0)

m_member8(NULL)
{
// Do something with the member1 value
memset(m_member3, 0, sizeof(m_member3)) ;
m_member4[0] = 0 ; // Character array
memset(m_member6, 0, sizeof(m_member6)) ;

}
Foo(some args)
: /* … here we go again … */

Damn you, Stroustrop! :)

14 Comments

I can’t understand any of that. You know why?

Claypit
Beef Vindaloo
Chicken Biryani
Cream Cheese Naan

Yes, I’m just making you jealous.

I hate C++. That’s all you need to know, Bloo, C++ is the devil. Hell is written in C++.

Bloo, try copy pasting all of the little code snippets (including both the before and after versions too) Oliver has posted on his blog, just slapping them on a single file and sending it to Dana for building a new version.

Oh, and web 2.0 :)

A C++ Class with hundred of members???
Could it be, that you’re doing something wrong? I always learned and still read it everywhere, that you shouldn’t have more than 10-20 members per class. If you have more, then it is about time to split the class in others.
A class in C++ always should only have one task. As soon, as you have an “and” in the definition of the tasks of your class, you should split it up.

And why do you use a static_cast for the upcast? The upcast should be executed automaticly by the compiler.

@ahwulf,
I love C++, the heaven is probably written in C++, the hell is written in C. And the earth in Java :)
If you once got the trick behind C++, you can fast write really powerfull programms. The problem is, to get the trick. C++ is very hostile to newbies :)

@kfs1,
Stroustrup :p
http://www.research.att.com/~bs/bs_faq.html#pronounce

Drave

You do know that multiple inheritance is evil, right?
Sure it is a fix for numerous problems, but just like many other things in C++, it isn’t *elegant*. Be wary or you will regret it in the end… :-)

Pokute: you did read the first sentence, right? :)

In this particular case, it /is/ elegant. It’ll get replaced with a struct with two members later on, but this allowed me to replace a structure that /manually/ comprised the two structures member-by-member with inheritance. And the structure has no purpose except to provide a struct {} to contain the data for transfer between host processes.

Drave: As I mentioned, most of them are “functional structs”. While, yes, it would be nice if, say, the vehicle structure was split into nice, tidy, compartmentalized structures, I’m not sure how healthy it would be. Short of rewriting everything you’d wind up with a nightmare of cross-references that would be counter productive rather than beneficial to productivity.

I’m not advocating huge classes – but when you are refactoring code, you have to be a little more practical than academic.

Isn’t the difference between C++ Classes and structs simply that structs are default public (including methods) and classes are default private?

Not sure if structs deal with virtual functions though.

The question is, what will be in the future? Won’t it become more and more a nightmare to maintain the code? Because now you receive something between C, C++ and perhaps something undefined :)

You already start to encounter some problems:
– Huge class, huge constructor
– memset, vtable

I’m pretty sure, there will be more soon. You shouldn’t mix up different concepts or you receive a chaos.

Someone, in the C++ Forum I am, once said, when you want to refactor a project from language A to language B, you should restart from zero.
I know, this isn’t very profitable, but perhaps only in the short term. In the long term a clean project would be helpful.
I want to say it clearly, I don’t have much experience in such things. But personally, I would continue with the old code base and start a new side project with a new code base, new ideas, new language, from zero. And perhaps you will then be able to swap bigger parts or even better the whole code. You never encounter something between and always have a strict separation.

Drave

This http://www.neilgunton.com/doc/rewrites_harmful has some insights on why rewrites are rarely a good idea.

Netscape -> Mozilla: 4-5 years with no new products, dwindling market shares etc.

Im just glad I have someone like oli working on a project that I hold dear, The fact that most of us have no clue of what this stuff means, I trust that oli has a clue because of his past work on this project. If I should ever find myself near RAT HQ It would be my pleasure to treat you guys to a beer.

I have learned a little reading this blog but I try and visit often because its entertaining and informative. I get a feeling of being on the inside on things while reading the post here. A free look into whats going on with how the game is progressing. Dont ever stop using this blog to post things like the above even if some of us dont have a clue who read it because we still enjoy the reading. :)

Im with rebel

All you need is a framework rib to provide the member ton structures with class-wide gone references to a pointer tree foo, which will allow the creation of constructor party class escalator members with very little overhead plunker.

Bah, why bother with all this C++… a few dozen lines of perl and Bob’s yer uncle!

;)

Soraz: The /only/ difference between classes and structs is that class members are private by default. But, if … I repeat: the /only/. And it’s best to try and remember it that way around because if you decide to go ahead and add a constructor to a struct, it’ll work. If you go ahead and add a constructor to a class, you’ll probably need to go ahead and remember to add the “public:” statement or it won’t work.

Drave: Whomever said “refactor” and “from zero” was probably also saying a lot of other important things, because you don’t “refactor from zero” – that’s called “rewriting”.

You also missed a subtle nuance. You picked up on this part

C++ constructors are crap: maintaining constructors for a class with a hundred members is a pain, and then you can’t initialize arrays or char arrays anyway. A good old memset is far more to the point and efficient, just fatal if you’re using classes with vtables.

But you failed to read it in the context the sentence started:

Perhaps my biggest obstacle was that most of our classes initialize themselves with a memset.

We don’t have constructors with 100s of members. Many of our structs are huge. Some of them have been converted into classes primarily as a way of denoting “this now has member functions”.

Rewriting our codebase is not an option: We’re not a box product company, we’re a software-as-service business and that means that up and rewriting the entire project can only be a goal that must be achieved through continuous iterations of refactoring.

The combined client/server codebase is 650,000 lines of code not including things like the UI or 3rd party libraries or the network stack.

I would hazard a guess that it would take 1-2 years dedicated work for the 4 of our coders to rewrite the game from scratch, making judicious use of modern 3rd party libraries and tools. Of course, that estimate is wrong by the amount of time we’d need to familiarize ourselves with those libraries and tools sufficiently, make mistakes with them and learn how to get it right for the final thing.

That’s dedicated work, and would get us to “where we’re at + some features + improved performance + improved graphics”.

If it was being done as a side project, reaching that same goal is going to be a 4-6 year project. During which the actual product will have changed, evolved and adapted, so your 4-6 years of work would be 4-6 years behind the product.

That doesn’t seem a particularly practical approach to development.

I think our process of refactoring, while not a universal ideal, is actually working out pretty well. It is a continual slave to the fact that we can’t down tools and just do whatever needs doing; it has aide the development in process rather than dictate it.

This case of multiple inheritance is a perfect example: it allowed me to distill out the two distinct sets of data which had been ground up into one structure without requiring me to reprogram for the new scenario.

Leave a Reply

Name and email address are required. Your email address will not be published.

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

You may use these HTML tags and attributes:

<a href="" title="" rel=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <pre> <q cite=""> <s> <strike> <strong> 

%d bloggers like this: