1 Guidelines for Writing Empire Code
3 compiled and edited by Markus Armbruster
8 Empire is an old program, and the age shows in places. It took a lot
9 of people a lot of effort to write it. Chances are that some of them
10 were smarter than you and me. Treat the code with the respect it
13 These guidelines are just that: guidelines. Not law. There's no
14 Empire Coding Police. People hacked on Empire code long before I knew
15 it existed, and I hope they still will when I fade into Empire
16 history. But there's substantial experience behind these guidelines,
17 so don't dismiss them cavalierly.
19 Here's our goal: algorithmic clarity. All the rules and guidelines
20 are subservient to it. If you'd have to obfuscate your code to
21 satisfy the letter of some guideline, don't!
23 Mind, I said `clarity', not `self-realization' or `expression of your
24 individuality'. Dave Pare wrote:
26 Be invisible. When you make a change, think not about marking your
27 place in history, or about showing off how much nicer your two-space
28 tabs are than those old, icky eight-space tabs that the stupid
29 empire hackers of old used, but think instead about the aesthetics
30 of the whole. The resulting lines of code should flow smoothly from
31 the beginning of the procedure to the end. Empire is 60,000 lines
32 of code. If you're the general case, you're only changing fifty
33 lines, so be aware of that.
35 Some guidelines are more serious than others. When I use words like
36 `is', `shall' or `must', I mean it. When I write `should', it's a
39 Many guidelines concern style, i.e. they involve matters of taste.
40 That's how it has to be. Uniformity of style is important for
43 These guidelines don't attempt to be exhaustive. More complete
44 guidelines that are mostly compatible with Empire can be found at
45 <http://www.jetcafe.org/~jim/c-style.html>.
47 See also doc/contributing.
50 Source tree organization
51 ------------------------
59 In My Egotistical Opinion, most people's C programs should be
60 indented six feet downward and covered with dirt."
63 Over the years, enough Empire coders lacked the good taste to preserve
64 the style of the original Empire code in their changes, and thus
65 turned the source code into an unreadable mess. In 2003, we fed it to
68 We tried to restore things to the original style, mostly. There is
69 one notable change: basic indentation is now four spaces. Restoring
70 the original eight spaces would have resulted in many more long lines,
71 which would have to be broken by indent. Since indent isn't good at
72 breaking lines tastefully, we reluctantly chose four instead.
74 FIXME mention src/scripts/intend-emp, even better: convert it into an
75 indent profile and mention that.
77 If you use Emacs, `stroustrup' indentation style produces satisfactory
78 results. The command `c-set-style', normally bound to `C-c .', lets
79 you set the style for the current buffer. Set variable
80 `c-default-style' to "stroustrup" to switch to this style for all
83 Avoid gratuitous space change
85 Don't change whitespace gratuitiously, say just because your editor
86 screws up tabs. Such changes make it much harder to figure out who
87 changed what and when.
91 Whether you use TAB characters or not doesn't really matter that much,
92 but TAB stops are every eight characters, period.
94 Indentation, placement of braces, function name
96 Basic indentation is four spaces. The opening brace goes on the same
97 line as `if', `struct', etc. Put `else' and do loops' `while' one the
98 same line as the closing brace. You are encouraged to leave out
99 syntactically optional braces. Don't indent case labels.
119 In a function definition, the return type, function name and the
120 braces must all start on a new line, unindented, like this:
128 This does not apply to function declarations.
132 Line length should not exceed 75 characters. Break such lines at a
133 logical place, preferably before an operator with low precedence.
134 Line up the continuation line with the innermost unclosed parenthesis,
135 if there is one, else indent it four spaces.
147 Use blank lines to separate different things. Functions must be
148 separated by a blank line. You are encouraged to insert a blank line
149 between a block's declarations and statements.
153 There is a space after `for', `if', and `while'. If `for' and `while'
154 are followed by `;' (empty loop body) on the same line, there is a
155 space before the `;'. There is no space between the function name and
156 the `(', but there is a space after each `,'. There should be no
157 space between type cast operator and operand. There is no extra space
158 after '(' and before ')'.
162 for (p = s; *p; ++p) ;
163 printf("%ld\n", (long)(p-s));
167 The function comment describing what a function does goes directly
168 above the definition.
170 Comments to the right of code should start in column 32 if possible
171 (counting from zero).
173 Comment lines should be indented exactly like the code they belong to.
175 You are encouraged to format multi-line comments like this:
178 * Please use complete sentences, with proper grammar,
179 * capitalization and punctuation. Use two spaces between
183 But please avoid elaborate boxes like this:
185 /***********************************************************
186 * Such decorations are a waste of time, hard to edit, and *
188 ***********************************************************/
194 Conditional compilation
196 Unless the conditional code is very short, please comment it like
215 DoNotUseStudlyCaps! Stick to lower_case and use underscores. Upper
216 case is for macros and enumeration constants.
218 File names should not differ in case only, since not all file systems
219 distinguish case. Best stick to lower case. Avoid funny characters
220 in file names. This includes space.
224 Like many good things, the preprocessor is best used sparingly.
225 Especially conditional compilation.
227 Do not use the preprocessor to `improve' the syntax of C!
229 Macro names should be ALL_CAPS to make them stand out. Otherwise,
230 they can be mistaken for objects or functions. This is bad, because
231 `interesting' behavior can hide inside macros, like not evaluating
232 arguments, or changing them. Exception: if a function-like macro
233 behaves exactly like a function, then you may use a lower case name.
235 Parameters that can take expression arguments must be parenthesized in
236 the expansion. If the expansion is an expression, it should be
237 parenthesized as well.
239 You are encouraged to use enumeration constants instead of macros when
240 possible. The existing code doesn't, but it makes sense all the same.
244 Every file should have a file comment FIXME
246 Every function should have a function comment that describes what it
247 does, unless it's blatantly obvious. If the function is longer than a
248 few lines, it's almost certainly not obvious.
250 The function comment should serve as a contract: state the
251 preconditions, side effects, return values. Make sure to cover error
254 Writing such comments helps both future maintainers and yourself: when
255 writing a concise function comment is too hard, then your function is
256 likely too complicated and could use a redesign.
258 Use @param to refer to parameters. Use func() to refer to functions
259 or function-like macros. Use arr[] to refer to arrays. This is to
260 make the references stand out and to avoid ambiguity.
265 * Read update schedule from file @fname.
266 * Put the first @n-1 updates after @t0 into @sched[] in ascending order,
267 * terminated with a zero.
268 * Use @anchor as initial anchor for anchor-relative times.
269 * Return 0 on success, -1 on failure.
272 read_schedule(char *fname, time_t sched[], int n, time_t t0, time_t anchor)
274 When documenting a struct or union, use @member to refer to its
277 The existing code has very little useful comments, and it'll likely
278 take us years to fix it. Please don't make it harder than it already
283 Do not declare system functions yourself; include the appropriate
286 Use prototypes, not old-style function declarations.
288 To get the best use of C's type checking, each function or variable
289 with external linkage should have exactly one declaration, in some
290 header file, and that declaration must be in scope at the definition.
291 No other declarations should exist. In particular, please include the
292 appropriate header instead of just declaring stuff again. The code
293 used to be full of this kind of junk, and it was quite some work to
296 Forward declarations of static functions should all go in one place
297 near the beginning of the file.
299 If you want a typedef name in addition to a structure or union tag,
300 give it the same name, like this:
302 typedef struct foo foo;
304 Yes, this is incompatible with C++. Reducing the number of names for
305 the same thing is more useful than compatibility to a programming
306 language we don't use.
308 Please don't hide away pointers with typedefs, like this:
310 typedef struct foo *foop;
312 When I see `foo *', I *know* what it is. When I see `foop', I have to
317 Do not use increment operators to set a variable to logical true! It
318 obfuscates the purpose, and narrow variables could even overflow.
319 Just assign 1. A lot of cleanup remains to be done there.
321 Null pointer constant
323 Please use NULL for clarity, not just 0.
327 Casting pointers to and from `void *' clutters the code and serves no
328 purpose (repeat after me: C is not C++). It is also unsafe, because
329 the cast suppresses type warnings.
331 Casting function values to void clutters the code and serves little
332 purpose (repeat after me: C is not Pascal).
334 Casting an argument to the type of the parameter clutters the code and
335 serves no purpose (repeat after me: ISO C is not K&R C).
337 Factor out common code
339 Do not gratuitously duplicate code! Ken Stevens said it well, and
340 it's as relevant as ever:
342 Cut-and-Paste coding is by far the biggest problem that the current
343 Empire suffers from. This is how cut-and-paste coding happens. Joe
344 Shmuck decides that he wants to add a new function to the server.
345 So he goes hunting through the server to find some already existing
346 code which does something similar to what he wants to do. This is
347 good. You should always write new code to imitate old code. What
348 is bad is when Joe Shmuck decides to simply "copy" 200 lines of code
349 from the old function into his new function, and then just change a
350 couple of little things here and there to suit his needs. This
351 method, known as Cut-and-Paste coding is the fastest and easiest way
352 to code. However, it results in a server that is impossible to
353 maintain. What Joe _should_ have done, is "move" the 200 lines of
354 code into a new _third_ function which the first two both call.
355 This is called writing a "general solution" to handle both cases.
356 Most of my work in the Empire2 project consisted in cleaning up
357 after a bunch of Joe Shmucks.
363 Code for IEEE Std 1003.1-2001 (POSIX.1-2001) with the X/Open System
364 Interfaces Extension. This standard includes ISO/IEC 9899:1999 (C99)
367 Some systems may have flaws that require work-arounds. Use Autoconf
368 to detect the flaws, and isolate the system-specific code, e.g. by
369 providing a replacement function when the system's function is flawed.
371 Keeping the Windows build working may require extending src/lib/w32/.
373 Keep the client as portable as practical. In particular, stick to
376 FIXME sizes, printf formats
380 FIXME conditional compilation is a last resort
388 Check error conditions meticulously. The existing code is bad enough
389 at this, please don't make it worse.
391 FIXME what to do on error
395 In many places, the code checks for conditions that should not happen,
396 and then tries to recover. This is sound defensive programming.
397 Please use CANT_HAPPEN() and CANT_REACH() for this purpose, because
398 they log the error condition, and optionally abort the program, or
399 write out a core dump. This is called an "oops".
407 Thread stacks cannot grow. If you use too much automatic storage, you
408 can overrun the stack with disastrous consequences. FIXME elaborate
410 Empire uses non-preemptive threads: a thread runs until it yields the
411 processor. This simplifies programming, but there are still traps to
412 avoid. Yielding the processor is an explicit thread operation, and
413 whether a thread operation yields is documented in empthread.h.
414 However, the operation may be buried in library code.
416 In particular, player input may yield. Player output may yield only
417 if it goes to the current player, and his last command doesn't have
418 the C_MOD flag. You can use this to simplify command code: set C_MOD
419 if you don't want to worry about yield on output. This is commonly
420 done for commands that modify game state.
422 Be careful with static storage. Read-only data is fine. More than
423 one thread writing static data is problematic, but there are safe
424 uses. For instance, a static scratch buffer that is never used across
427 Yielding the processor invalidates *all* the game state you copied
428 into variables. You have to reread and possibly recheck. See below.
432 Game state consists of several sets of objects, e.g. sectors, ships,
433 bmaps, ... Each set is fully held in memory and backed by a file in
436 There are several ways to access an object in set FOO:
438 * You can get a pointer to the object (not a copy!) with getFOOp().
440 This is inherently dangerous, because it allows you to update the
441 object in memory without updating the disk file. It's used mostly
442 in update code, where it pays major performance dividends, but there
443 are uses in other places. Whether they are wise is debatable.
445 Obviously, the object can change when you let another thread run.
446 This is not an issue in update code, because it never yields.
448 * You can get a copy with getFOO() and write it back with putFOO().
450 Putting updates both the object in memory and the disk file.
452 Any change to the object invalidates the copy. Putting such an
453 invalid copy will clobber the change(s) that invalidated it,
454 possibly resulting in corrupted game state. The code oopses on such
455 puts, but it can't repair the damage.
457 There are two common ways to invalidate a copy: calling a function
458 that updates the object you copied (unless it does that through your
459 copy), and yielding the processor, which lets other threads update
460 the object you copied.
462 Therefore, you have to re-get after a possible invalidation, and
463 deal with changes. In particular, if you checked whether the object
464 is suitable for a task, you need to check again after re-getting it.
465 If you can afford to bail out when something changed, use
468 Function comments should state what objects the function can update.
469 Unfortunately, they generally don't.
471 It's best to keep puts close to gets, both at runtime and in the
474 * Bmaps have special access functions.
476 Each country has two bmaps: the working bmap and the true bmap.
477 Unfortunately, the bmap code calls the former `bmap' and the latter
480 You update bmaps with map_set(). This doesn't write through to the
481 file; instead, it returns non-zero when the update changed anything.
482 In that case, you must write bmaps to disk with writemap() before
483 you yield. If you only updated the working bmap, then you can call
488 Reading player input can fail, and you must check for that.
489 Neglecting it can break the interrupt feature for players (normally
490 Ctrl-C), and produce extra prompts that could confuse clients. Even
491 worse is neglecting it in a loop that terminates only when input is
494 When reading input fails, you should normally abort the command with
495 status RET_SYN. There are exceptions, e.g. when aborting a pinpoint
496 bombing run over the target.
498 Some functions to read player input return a special error value you
499 can check, e.g. recvclient(), prmptrd() and uprmptrd() return a
500 negative value, getstring() and getstarg() return NULL.
502 Others return the same error value for failed read and for successful
503 read of input that is invalid. Then you need to check
504 player->aborted; if it is set after a read, the read failed.
507 Historical guidelines, superseded by the above
510 Remarks from Dave Pare:
512 And now, a few editorial remarks on my views on Good Techniques for
513 Modifying Existing Software:
515 My safari through the hot and bug-ridden jungle that Empire has become
516 has given me some new insights on the modification of large software
517 packages. My adventure has prompted me to propose some simple coding
518 guidelines for those dreaming of hacking on the code:
520 1) Be invisible. When you make a change, think not about marking your
521 place in history, or about showing off how much nicer your two-space
522 tabs are than those old, icky eight-space tabs that the stupid empire
523 hackers of old used, but think instead about the asethetics of the whole.
524 The resulting lines of code should flow smoothly from the beginning of the
525 procedure to the end. Empire is 60,000 lines of code. If you're the
526 general case, you're only changing fifty lines, so be aware of that.
528 2) Procedurize. If you find yourself in a triple-nested loop, or a five
529 hundred line procedure, perhaps it's because you need to split some of
530 that code off into a procedure. In general, if you've got more than two
531 levels of tabs (or indentations), or more than one or two major loops in
532 a procedure, something may well be amiss.
535 Sasha Mikheev on indentation:
537 The empire indentation style can be achived by using
538 indent -orig -i8 foo.c
540 or in old c-mode emacs (versions before 19.29):
542 (setq c-indent-level 8)
543 (setq c-continued-statement-offset 8)
544 (setq c-argdecl-indent 8)
545 (setq c-brace-offset -8)
546 (setq c-label-offset -8)
549 Further comments by Ken Stevens:
552 The only variables which should be global are constants. If you write
553 a routine which changes a global variable, then you will corrupt the
554 data when two different players run that routine at the same time.
562 Should be rewritten as:
568 AIX has different conventions for signed chars, and IRIX requires the
569 /* comments */ after #endif.
572 Cut-and-Paste coding is by far the biggest problem that the current
573 Empire suffers from. This is how cut-and-paste coding happens. Joe
574 Shmuck decides that he wants to add a new function to the server. So
575 he goes hunting through the server to find some already existing code
576 which does something similar to what he wants to do. This is good.
577 You should always write new code to imitate old code. What is bad is
578 when Joe Shmuck decides to simply "copy" 200 lines of code from the old
579 function into his new function, and then just change a couple of
580 little things here and there to suit his needs. This method, known as
581 Cut-and-Paste coding is the fastest and easiest way to code. However,
582 it results in a server that is impossible to maintain. What Joe
583 _should_ have done, is "move" the 200 lines of code into a new _third_
584 function which the first two both call. This is called writing a
585 "general solution" to handle both cases. Most of my work in the
586 Empire2 project consisted in cleaning up after a bonch of Joe Shmucks.
587 I took repeated code and "consolidated" it into general function
591 Just to add to Dave's "Be Invisible" motto, I'd like to give a little
592 example to illustrate some basic do's and don'ts for coding style:
594 The following function has bad style:
596 double att_combat_eff(com,own)
601 if(com->type==EF_SECTOR)
605 str=com->sct_dcp->d_ostr;
607 str=com->sct_dcp->d_dstr;eff=2.0+(str-2.0)*eff;
608 }else if(com->type==EF_SHIP&&com->own!=own)
609 eff=(1.0+com->shp_mcp->m_armor/100.0);
612 Here is the same function written with "good" style:
615 att_combat_eff(com, own)
622 if (com->type == EF_SECTOR) {
623 eff = com->eff / 100.0;
625 str = com->sct_dcp->d_ostr;
627 str = com->sct_dcp->d_dstr;
628 eff = 2.0 + (str - 2.0) * eff;
629 } else if (com->type == EF_SHIP && com->own != own)
630 eff = (1.0 + com->shp_mcp->m_armor / 100.0);
635 These are all the things I fixed when changing the bad to the good:
636 - Function names should always start a new line (so you can search for them)
637 - There should always be a space after a ","
638 - Function arguments should be indented 8 spaces
639 - There should always be a tab after a type declaration
640 - Opening function bracket should be on a line by itself
641 - Indentation should be 8 spaces
642 - There should always be a space on both sides of every operator
643 - There should always be a space after if, for, while, switch
644 - The opening bracket should be on the same line as the if
645 - There should always be a space on either side of a {
646 - There should always be a new line after a ;
647 - The closing function bracket should be on a line by itself