Sorry your browser is not supported!

For those who don't know what Lugaru is (very few people on this forum, I'm sure), here's the wiki:

http://en.wikipedia.org/wiki/Lugaru

The game's source code got released a little while ago (well, three years but still). There's more in this blog entry: http://blog.wolfire.com/2010/05/Lugaru-goes-open-source

Anyways, a friend of mine who has taken an interest in this had a look through the source and found it a little confounding (+10 for excellent quip). This is what he described in the email he sent me:



I was able to sorta follow it but I must admit, the code style isn't very consistent and it's definitely not the most human-readable thing ever. Here's just one random example (may require scrolling due to indentation): http://hg.icculus.org/icculus/lugaru/file/97b303e79826/Source/GameTick.cpp#l7276

So I was wondering what everyone else's take is?

Lemme just quote line 7201 from that file:


Yes, that is one line. Had a discussion with Alex (TheComet) too, and while I have less experience in C/C++ (mostly basic stuff) I too was baffled by the complete lack of comments, the nested 15 and more if-clauses (where all but the last one are lacking curly brackets... which will compile fine as the next line is just another if statement... but... that surely is not good style?) and other stuff.
Just look at this.

why would you nest these like this? Why not write a 3d distance function and write one humanly readable check?!

The GameTick file you linked seems to contain most of the game's code for whatever reason.

It also contains this part:

and so on. The ConsoleCmds.h contains lines like e.g. "DECLARE_COMMAND(quit)" which are in the above snipped warped by some crazy pre-processor magic turning it into a) an array of the names of the commands; b) prototype declarations (I think that's what they're called) for the ch_* functions; and c) an array of the function handlers.
It might work, but somehow I don't get why they couldn't just write those three things manually or shove them into the ConsoleCmds Header or whatever? Maybe it's just me not being used to C++ but that code srsly confused me for a moment before I actually grasped what it does.

The Terrain.cpp / Terrain.h have this stuff for example:



And they differ only marginaly and in some strange way. Nothing gets explained there.
Why not have one of these functions each and have some macro-switch or a flag as an argument to change the behaviour of the function (and name it sensibly!)...

But, yes, I'll admit I have had code like this before too. Worse even. Didn't undestand my own project after a while because it got cluttered with testing bits and tweaks and hacks. But why for ponies sake would they then just throw the source out there? It is quite the enigma indeed...

Oh and kind regards from TheComet, who as I mentioned showed me this too a while ago.

Oh and he said that they're "using namespace std;" which as he told me is pretty darn horrible coding in itself. (As I said my C knowledge is limited and rusty...)

PS: It even had "if(1==1){" somewhere
PPS: Link just look at "bool Person::isIdle(){" ....

Edit: Actually nevermind about the way the Commands are handled. It was actually worse before it got sorted out and structured the way it is by a certain "Alexander Monakov" (dunno if he's part of Wolfire but I don't think so.), here's the changelog/diff for that. Before it was all parsed & executed manually inside "void Game::Tick()" as far as I can tell (that function handles all the keyboard-input, menues, animation-related stuff... imho it could all be split off into other parts of the code.
Wait. Wait. Waaait. GameTick() goes from line 2480 to 9435?! I hadn't realized 75% of the 10k lines of code in that file are
inside that one function... and it seems to handle basically ALL game mechanics... holy mother of code. With the commands in it too it must've been ONE big 10k lines function. I don't even.

Only if you are using it in a larger scope.

Oh yes I meant to say they're using the std namespace for EVERYTHING. (atleast I think they did, or for large parts of the code.)

Edit: It is at the top of GameTick.cpp atleast.

Its easy to point the finger at someone's code and point out its shortcomings. But these can be for a number of reasons:

1. Lack of experience
2. Tight Deadlines
3. Working by yourself

At the end of the day Lugaru was a finished product. The person who wrote this code worked by himself and if it was his first commercial game most likely did it in a hurry!

I don't disagree that this code isn't messy, we're all guilty of writing stuff like this, and if people wrote code like this when working with others it would really slow down development.

But it might be worth bringing something constructive to the table and using this as an example, rather than a point and laugh exercise.



For instance. From my experience, code can quickly turn into something like this if you don't properly plan out your algorithms/data models before you actually start to code.
This sort of code can also happen if you don't plan any flexibility in your software design for change, and you are required to quickly make rather large significant changes in the future to your code.

This usually boils down to either Lack of experience or lack of time.

If its the former, my advice would be to establish on paper what exactly it is you are trying to achieve. Determine what your data models are going to look like. Speak to your employer/project manager about any possible requirements changes that could likely occur in the future, and determine from your initial plans where things need to be hardcoded and where things can be made flexible. Use a sensible design paradigm like MVC suited to the task and for the love of god use DRY (don't repeat yourself).

Its also worth setting out a set of coding conventions with yourself/your team.

For instance, the coding convention I've set out for work uses camel case for javascript (functionName) and underscores for PHP (function_name).

Its little things like this that if you stay consistent with them, your code won't actually need commenting.

(I'm on annual leave at the moment, and I've just checked the GIT recently and some of my colleagues have been changing my code, which they have never seen before. I haven't left any comments on my code but its set out in a certain way that follows a 'story', so they've figured out exactly what bits to change)

There are books written on writing effective code, and some of them are well worth a read!


If you are writing code like this due to lack of time, then its somewhat forgivable. Time can be made in the future to refactor your code!

Cheers,
BiggAdd

Never heard of it. yeah.. looks a bit of mess, but then again, most things look that way when you peer into them from the outside.

@RedFlames: You can pass my regards back to Alex if you speak to him again soon. He also asked me to pass on his regards to everypony but thought it might get the thread locked -- which it seems not to. Yaaay, we're allowed to talk about the people involved as long as we don't talk about *that.

@Seditious:

I agree in theory but to me personally, I've always seen using any namespace -- std or otherwise -- as a potential for collision and as almost defeating the purpose of enclosing anything in a namespace. My C++ knowledge isn't all that great so I'll stand corrected, just my personal feelings.

@BiggAdd: I agree with you about the finger pointing. I certainly wouldn't want people to see some of my earlier code, let alone even some of my recent code. I guess that's the main reason I keep my stuff closed-source. Still, if you're going to have people viewing your code and it's a mess like Lugaru, you should at least comment it and format it, even after the fact, even if it's only a little each week. It's been like 3 years. Also, I feel that if one puts one's source out there, one invites criticism (constructive and otherwise).

I'm a newbie and I think my code is quite darned clean ._.' but then, i'm not doing super complicated stuff xD But I just like to have an easy time finding everything, in the long run - it makes it easier for ME and if (in my case) I ask someone for help.