Game with voxels

I started a hobby project which would be the first thing I do in C that is intended to be seen by someone else. I've been practicing for a while but I'm actually a beginner; I never wrote real software in C/C++ yet.

I wanted to know what is OK from my code, what is wrong and why, if it's clear/readable, and everything you can tell me about it would be fine. I have never shown any of my code to anyone and I have no clue if I'm doing well, or if it can't be worse--the latter being what I consider most likely. So I would like to know the reasons from people with experience with C/C++, and what you consider that I can improve/remove, etc.

The code is something I've just started, about a small voxel system for a Minecraft-like game.

The basic idea would be:

• to have an array of voxel types

• to be able to assign rules to the voxel types (like functions/scripts to be called.. somehow?)

• to have an array of game relevant states for each voxel. The state data would be mostly lighting and visibility values.

• to be able to save and load state data as necessary

etc... I have a lot of ideas but I still barely wrote some code and I would like to know other people's opinions before continuing, so I can improve the project instead of having to fix it later.

This is the link for the project files (only a .h and a .cpp, it's just started) at google code

And here are some of the contents of the .h so you can give yourself an idea without opening the link:

```
typedef struct _GVOXEL_RULE
{
GVOXELRULE_ID nRuleID;
INT32 nRuleValue; // this value is left for use by the rule function, not needed right now
} GVOXEL_RULE;

// here I'm planning to store the description of the voxel types
typedef struct _GVOXEL_DATA
{
GVOXEL_TYPE nVoxelType;
dword nRuleCount;
GVOXEL_RULE* pVoxelRules; // I'm thinking this should be separated to a "voxel rule manager" class or something similar

Design

It looks like you have a design in mind, but it isn't all visible from the code posted.
I'm anyway a little hazy on what you're really trying to model here, so let's step through it.

• 3D array of voxel instances: this is your actual world state, got it.

• nVoxelType is ... an index into some type array not shown? Well, we clearly need some connection between a voxel instance and it's type, however implemented

• now, what properties does a voxel type need? You show an array of rules, but it isn't clear how they interact with the type. If you're intending that each rule should represent something like hit a voxel of type 7 with a pickaxe or set fire to a voxel of type 3, I'm not sure a linear array will cut it.

It might help to clarify things if you try sketching a game event: going from coordinates to a voxel instance is easy, but once you have that, what are you going to do to it?

Implementation

You say you probably want C rather than C++, but you can use similar polymorphism in either with a little effort, so designing the interaction is still more important than language choice.

Eg. C++

class VoxelType
{
public:
    // common interface for all types
    virtual void hitMeWithAPickaxe(Voxel *self) = 0;
    virtual void setFireToMe(Voxel *self) = 0;
};

class DirtVoxelType: public VoxelType
{
public:
    // specific implementation for this type
    void hitMeWithAPickaxe(Voxel *self);
    virtual void setFireToMe(Voxel *self);
};

or in C:

struct VoxelType {
    int type;
    void (*hitMeWithAPickaxe)(struct Voxel *self);
    void (*setFireToMe)(struct Voxel *self);
};

void hitDirtWithAPickaxe(struct Voxel *);
void setFireToDirt(struct Voxel *);

struct VoxelType VoxelTypeArray[] = {
  { DirtType, hitDirtWithAPickaxe, setFireToDirt },
  ...
};

Obviously you can also use the time-honoured massive switch/case statement in either language too - the point is just that the selection of C or C++ isn't the driving force behind the design, the design is the driving force behind the implementation.

volatile long nRefCount; // <-- This gives me the chills
I don't blame you - even if you already know your game is going to be multithreaded, I can't see this working. Either design concurrency and synchronisation in up-front, and do it properly, or omit it entirely and be prepared to do some re-design work later if one thread really isn't good enough.

Style

In approximate order of increasing subjectivity

What is up with the #define gvSaveChunkToFile gvSaveChunkToFileW stuff? Is this intended to ease some cross-platform stuff later, or did you just copy it from elsewhere? IMO it's ugly and potentially confusing, so I'd remove it unless it adds something really worthwhile I can't see here.

Likewise with all the GVOXEL_STATE_1_0, GVOXEL_STATE stuff - unless you're really planning to have, and deal with, multiple incompatible versions of the same structure at runtime (which sounds horrible), you can isolate any backwards-compatibility logic to the (de)serialization layer and remove the duplicate names in your main code.

You're using lots of (what I think are) platform-specific typedefs, in case you care. Maybe byte, dword etc. are idiomatic on that platform so there is a big consistency benefit, but I'm not clear what other advantage they have over the standard types like char, int8_t or uint8_t.

Subjectively I'm not a big fan of Hungarian notation as used here: you're just replicating the type and not adding any semantic information.

Finally, and again subjectively, I don't like ALL_UPPER_CASE type names or _LEADING_UNDERSCORE_UPPER_CASE symbols, they get perilously close to the __RESERVED_NAMES and just tend to look ugly.
I also don't see the need to typedef every struct; struct VoxelState instead of GVOXEL_STATE_1_0 seems clearer and easier to read

StackExchange Code Review Q#6419, answer score: 5