AC Blog-ish

[Mousikos]

So some people may be wondering why I'm bothering with this. Maybe it's a waste of time, maybe it isn't. Well, here is why I'm doing this.


-Code is made to be read by humans.-

The compiler may not perfectly translate your code to the most efficient machine code. In fact, even though compilers are getting better and better, if you want really efficient code, you should probably write a lower level than C++. Furthermore, you'd have to write for different architectures: x86, x86_64, amd64, MIPS, etc. Since most code, specifically Assaultcube, is meant to function or more than one system, it is advantageous to write it in C++ and let the compilers handle the tedious work.

-Humans are not perfect-
Have you ever messed up coding? I sure have. I spent hours looking over code trying to find the error and failed. I had to rewrite it. What a waste a time. Instead of trying to cut corners and leave out comments as well as not sectioning my code, I could have coded correctly. I may know what I'm trying to accomplish right now, and I may know exactly what my code does. But will I know it three hours down the line? How about three days or three months or three years?

What if I'm working with other people? Everyone has a different style of code. What comes easy to you may not come easy to other people. Humans, unfortunately, have also not evolved to the point where they can read mids. So, tell people what your code does! If someone discovers a bug that could be in your code 10 years down the line, make it easy for them! Section off your code! Tell people what you did!

-Standarize to make things easier-
This goes hand in hand with the fact that everyone thinks different. Come up with a standard, and /stick with it/. Don't be the one guy who instead of using the standard:

[SELECT ALL] Code:

/*************************************************************************
* Function: generatesound
* Description: Generates sound from a 3D point source
* Input: file - sound file to be played, coord_vec: Vector of the coordinates of the source
* Output: -
**************************************************************************/

You go and write your own shit:

[SELECT ALL] Code:

//Generates sound

If that's you, you are an asshole and you should feel bad.

But why is this worth mentioning at all?

Assaultcube is an OPEN-SOURCE PROJECT. That means anyone, not just our current set of devs, can come in and edit some things. Of course, there are some things that they can't use -- the images, the maps, the sounds, etc. Things that are copyright protected. But they can do whatever they want with the code. Therefore, we should try to encourage collaboration instead of writing spaghetti code.

The current state of assaultcube is spaghetti code. This is the current state of sound.cpp:

[SELECT ALL] Code:

// sound.cpp: uses OpenAL, some code chunks are from devmaster.net and gamedev.net


#include "cube.h"

It has a comment describing what the file is supposed to be -- and then contains NONE of the aforementioned content. It just includes a header file!



Yes, you read that right. Don't believe me? Here you go: https://github.com/assaultcube/AC/blob/m.../sound.cpp

This commit was made in 2009! No wonder AC had a sound bug! Here are the list of files in AssaultCube:
http://sprunge.us/EUHG

Knowing that openAL is something to do with sound, where would you look if you had a sound bug? My vote goes for the following files: audiomanager.cpp, oggstream.cpp, openal.cpp, sound.cpp, sound.h, soundlocation.cpp, soundscheduler.cpp, stream.cpp.

But audiomanager.cpp is clear some random code

[SELECT ALL] Code:

#define DEBUGCOND (audiodebug==1)

VARF(audio, 0, 1, 1, initwarning("sound configuration", INIT_RESET, CHANGE_SOUND));
VARP(audiodebug, 0, 0, 1);
char *musicdonecmd = NULL;

VARFP(musicvol, 0, 128, 255, audiomgr.setmusicvol(musicvol));

and then the definition of the audiomanager class! So where's the header file? Where is the class definition? I want to take a better look at it. There's no audiomanager.h. Oh, okay. It's buried in sound.h. But sound.h has more class definitions, and then some random code.

[SELECT ALL] Code:

void alclearerr();
bool alerr(bool msg = true, int line = 0, const char *s = NULL, ...);
#define ALERR alerr(true, __LINE__)
#define ALERRF(fmt, ...) alerr(true, __LINE__, fmt, __VA_ARGS__)

extern vector<soundconfig> gamesounds, mapsounds;
extern ov_callbacks oggcallbacks;
extern int soundvol;
extern int audiodebug;

extern audiomanager audiomgr;

Like I said before, I'm not a great programmer, but it sounds like most of that should go somewhere else. Perhaps in sound.cpp?

Okay, okay. That can be dealt with. But wait. Does audiomanager.cpp include cube.h? Hey, so does client.cpp... and clientgame.cpp and clients2c.cpp and command.cpp and console.cpp and crypto.cpp and docs.cpp... okay you get the point.

At this point, it really seems like most of the code is in cube.h. Cool. I guess. But then, hello cube.h:

[SELECT ALL] Code:

#include "platform.h"
#include "tools.h"
#include "geom.h"
#include "model.h"
#include "protocol.h"
#include "sound.h"
#include "weapon.h"
#include "entity.h"
#include "world.h"
#include "i18n.h"
#include "command.h"

Well, the good news is (make the best of it, right?) at least the authors had the foresight to write

[SELECT ALL] Code:

#ifndef __CUBE_H__
#define __CUBE_H__
#endif

Did I mention the sparse commenting? Also, a lot of variables are like e, s, r, etc. Saves the programmer trouble when doing it, I suppose. But wouldn't it be better if they were given some meaningful name?

By the way, somewhere in the source there is a doxygen folder...

Maybe I'm not that familiar with doxygen, but I don't see much doxygen like formatting: http://www.stack.nl/~dimitri/doxygen/man...locks.html

Well, the doxygen folder's last commit was four years ago so...

Anyway, enough rambling. That's why I'm doing what I'm doing.

Cheers,
Mousikos

PS: I completely understand not wanting to have like 5 files with 5 lines each in them. I might do it too. But then, I might put all the stuff sound.h mentions into sound.cpp. Just saying.

---

Also:

[SELECT ALL] Code:

server.cpp:2218:139: warning: address of array 'v->action->desc' will always evaluate to 'true' [-Wpointer-bool-conversion]

    logline(ACLOG_INFO, "[%s] client %s called a vote: %s", clients[v->owner]->hostname, clients[v->owner]->name, v->action && v->action->desc ? v->action->desc : "[unknown]");
                                                                                                                            ~~ ~~~~~~~~~~~^~~~
server.cpp:2225:152: warning: address of array 'v->action->desc' will always evaluate to 'true' [-Wpointer-bool-conversion]
    logline(ACLOG_INFO, "[%s] client %s failed to call a vote: %s (%s)", clients[v->owner]->hostname, clients[v->owner]->name, v->action && v->action->desc ? v->action->desc : "[unknown]", voteerrorstr(error));
                                                                                                                                         ~~ ~~~~~~~~~~~^~~~
server.cpp:3498:56: warning: address of array 'vi->text' will always evaluate to 'true' [-Wpointer-bool-conversion]
                            char *vmap = newstring(vi->text ? behindpath(vi->text) : "");