Coding guide

[davidjoffe]

Coding guide

IMPORTANT: Although the old code is/was full of non-secure functions like "sprintf" and "printf" please do not add more of these (all these old ones are in the process of being fixed/updated to safer alternatives)

NB NB! If anything 'big' you want to change (or anything that might not be in tandem with my own 'vision' for the project - please discuss first - and/or do in either a fork or a branch - or any non-trivial changes, also suggest use a branch so I can review the changes and check I'm OK with them etc. before merging into main.

Please remember when committing any fix that it may affect other platforms.

Small fixes that seem obviously you can probably just commit directly. If in doubt, use a branch (or fork) and let me check - thanks

Guidelines:
Personally I'm not a big fan of templates ... quickly load to slow compiles and bloat it not careful and often inefficient - I prefer to keep template usage firstly (1) to a minimum necessary, and (2) in general try keep their usage with a .cpp class 'backend'/implementation where possible (rather than in a .h) so we can keep compile speeds quick. In certain cases they can be useful for performance or convenience.

Headers: Try keep headers as 'small fast light' as possible with minimal includes

Variable initialization: In general always make sure variables. For gamedev for performance in performance-sensitive parts of the code it's occasionally useful to deliberately leave some things uninitialized as long as one carefully codes in a way that you know you won't use the uninitialized value but if you ever do this MAKE A COMMENT marking that are you deliberately leaving it uninitialized for performance reasons.

Optimization: Usual rules apply, e.g. do profiler-guided optimizations etc. - don't focus optimization efforts on something that isn't actually a bottleneck - your 'instincts' aren't always right

Intellectual property: NEVER commit anything (data or source code) that may violate Intellectual Property of our distribution. Always double and triple check before adding anything - if in doubt (i.e. if you aren't 200% sure something is allowed Intellectual Property wise), leave it out. Our source code is dual-licensed (GPL2 / MIT) but the data is a mix of open-source-accepted licensees like creative commons variations

[davidjoffe]

Safety!

• Watch out for 'gotchas' on some platforms like:
  snprintf("%d items for %s", arrItems.size(), sName.c_str());
  On some platforms one may find that e.g. "size" is 64-bit but int 32-bit and %d will thus pull 32 bits off stack but .size() may pass 64 bits on stack thus causing a possible crash on the %s for the string. And you may not notice it because you compiled and tested on a platform where e.g. both are e.g. 32 or 64-bit etc.
  Rather do:
  snprintf("%d items for %s", (int)arrItems.size(), sName.c_str());
  or some other solution.

• Never use the older unsafe versions of functions like "sprintf", use at least safer versions like "snprintf".

Also if something doesn't need to use formatting specifiers in the first place, don't use any kind of printf at all, e.g. for things like this:
djLog("%s", sName.c_str());
rather use:
djLOGSTR(sName.c_str());
to reduce risk of printf-related bugs.

[davidjoffe]

Thread-safety! Although we don't currently use threads, I suggest let's try keep the code in a state that we could possibly start doing some things in threads. So a few basics, if we look at a logging function like:

Watch out for some static scope things e.g.:

void log_message( const char * szFormat, ... )

Let's say you coded this to vsnprintf to a static buffer - without a mutex that static buffer would be a direct source of thread bugs as different threads log at same time.

Solutions to avoid this, either use e.g. say "static thread_local" or not static.

[davidjoffe]

Dependencies: Let's try keep dependencies to a reasonable minimum and try not to rapidly continually add lots of and lots of dependencies for every little thing, keep them as stable as reasonably possible so as not to keep disrupting porters, but on the other hand there will be times where we need/want to add dependencies to move forward

For example there are some things that are potentially 'very useful' that we should be considering adding - e.g. SDL_ttf for Unicode support, or e.g. SDL_image for (say) ability to load .png files etc.

[davidjoffe]

In general 'config.h' should be included first or near-first toward top of any file (as e.g. it could affect what other #includes are included)

We maybe also should consider splitting, as we have different conceptual 'levels' e.g. the 'gamelib' and the sdl wrapping parts and finally the game part itself - each conceptually could maybe in future have seprate configs but that's unnecessary overkill right now - let's keep it simple

[davidjoffe]

config.h ideally shouldn't include anything for to keep it 'small and light and fast'

[davidjoffe]

Keep in mind 'model/view/controller' conceptual separation while coding

[davidjoffe]

The oldest parts of the code have a lot of 'gross globals' and these are a nuisance sometimes causing e.g. name conflicts - have been moving the code away from 'gross globals'

[davidjoffe]

Conceptually the code is divided into a few dependency 'layers' (but some of these maybe need a rethink)

Originally in the '90s the conceptual design layering was something like:

[game code] ------> ["djgamelib"] ---------> [backend] -----------{different possible backends, e.g. in the old days now-deprecated-libGGI/DirectX implementations -> now obsolete as LibSDL2 effectively does this job}

"djgamelib" parts are meant to be generic and could be hypothetically re-used for different games

Now the conceptual design layering is a bit more like:

[game code] ------> ["djgamelib"] / [sdl-backend]

I'd like to maybe gradually refactor it to separate out more generic game, vs "Dave Gnukem 1 specific" code:

[DG1-specific] -> [generic game code] ------> ["djgamelib"] / [sdl-backend]

(For example, consider the current blocky viewport scrolling that's highly specific to DG1 and part of the retro nature ... hypothetically we might have something like a generic 'djViewportControl' base class, and maybe derive from that e.g. a "djDaveGnukem1ViewportControl" that dose the Dave Gnukem 1 blocky viewport scrolling)

In general try not to add dependencies the wrong way round, e.g. the "gamelib" parts (which start with "dj" in src files) should not 'know about' stuff in the game code design-wise.