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
48 Source tree organization
49 ------------------------
57 In My Egotistical Opinion, most people's C programs should be
58 indented six feet downward and covered with dirt."
61 Over the years, enough Empire coders lacked the good taste to preserve
62 the style of the original Empire code in their changes, and thus
63 turned the source code into an unreadable mess. In 2003, we fed it to
66 We tried to restore things to the original style, mostly. There is
67 one noteable change: basic indentation is now four spaces. Restoring
68 the original eight spaces would have resulted in many more long lines,
69 which would have to be broken by indent. Since indent isn't good at
70 breaking lines tastefully, we reluctantly chose four instead.
72 FIXME mention src/scripts/intend-emp, even better: convert it into an
73 indent profile and mention that.
75 If you use Emacs, `stroustrup' indentation style produces satisfactory
76 results. The command `c-set-style', normally bound to `C-c .', lets
77 you set the style for the current buffer. Set variable
78 `c-default-style' to "stroustrup" to switch to this style for all
83 Whether you use tab characters or not doesn't really matter that much,
84 but tab stops are every eight characters, period.
86 Indentation, placement of braces, function name
88 Basic indentation is four spaces. The opening brace goes on the same
89 line as `if', `struct', etc. Put `else' and do loops' `while' one the
90 same line as the closing brace. You are encouraged to leave out
91 syntactically optional braces. Don't indent case labels.
111 In a function definition, the return type, function name and the
112 braces must all start on a new line, unindented, like this:
120 This does not apply to function declarations.
124 Line length should not exceed 75 characters. Break such lines at a
125 logical place, preferably before an operator with low precedence.
126 Line up the continuation line with the innermost unclosed parenthesis,
127 if there is one, else indent it four spaces.
139 Use blank lines to separate different things. Functions must be
140 separated by a blank line. You are encouraged to insert a blank line
141 between a block's declarations and statements.
145 There is a space after `for', `if', and `while'. If `for' and `while'
146 are followed by `;' (empty loop body) on the same line, there is a
147 space before the `;'. There is no space between the function name and
148 the `(', but there is a space after each `,'. There should be no
149 space between type cast operator and operand. There is no extra space
150 after '(' and before ')'.
154 for (p = s; *p; ++p) ;
155 printf("%ld\n", (long)(p-s));
159 The function comment describing what a function does goes directly
160 above the definition.
162 Comments to the right of code should start in column 32 if possible
163 (counting from zero).
165 Comment lines should be indented exactly like the code the belong to.
167 You are encouraged to format multi-line comments like this:
170 * Please use complete sentences, with proper grammer,
171 * capitalization and punctuation. Use two spaces between
175 But please avoid elaborate boxes like this:
177 /***********************************************************
178 * Such decorations are a waste of time, hard to edit, and *
180 ***********************************************************/
186 because they are not portable C89.
189 Conditional compilation
191 Unless the conditional code is very short, please comment it like
210 DoNotUseStudlyCaps! Stick to lower_case and use underscores. Upper
211 case is for macros and enumeration constants.
213 File names should not differ in case only, since not all file systems
214 distinguish case. Best stick to lower case. Avoid funny characters
215 in file names. This includes space.
219 Like many good things, the preprocessor is best used sparingly.
220 Especially conditional compilation.
222 Do not use the preprocessor to `improve' the syntax of C!
224 Macro names should be ALL_CAPS to make them stand out. Otherwise,
225 they can be mistaken for objects or functions. This is bad, because
226 `interesting' behavior can hide inside macros, like not evaluating
227 arguments, or changing them. Exception: if a function-like macro
228 behaves exactly like a function, then you may use a lower case name.
230 Parameters that can take expression arguments must be parenthesized in
231 the expansion. If the expansion is an expression, it should be
232 parenthesized as well.
234 You are encouraged to use enumeration constants instead of macros when
235 possible. The existing code doesn't, but it makes sense all the same.
239 Every file should have a file comment FIXME
241 Every function should have a function comment that describes what it
242 does. FIXME elaborate. Writing such comments helps both future
243 maintainers and yourself: if it's too hard to write a concise function
244 comment, then your function is likely too complicated and could use a
247 The existing code has very little useful comments, and it'll likely
248 take us years to fix it. Please don't make it harder than it already
253 Do not declare system functions yourself; include the appropriate
256 Use prototypes, not old-style function declarations.
258 To get the best use of C's type checking, each function or variable
259 with external linkage should have exactly one declaration, in some
260 header file, and that declaration must be in scope at the definition.
261 No other declarations should exist. In particular, please include the
262 appropriate header instead of just declaring stuff again. The code
263 used to be full of this kind of junk, and it was quite some work to
266 Forward declarations of static functions should all go in one place
267 near the beginning of the file.
269 If you want a typedef name in addition to a structure or union tag,
270 give it the same name, like this:
272 typedef struct foo foo;
274 Yes, this is incompatble with C++. Reducing the number of names for
275 the same thing is more useful than compatibility to a programming
276 language we don't use.
278 Please don't hide away pointers with typedefs, like this:
280 typedef struct foo *foop;
282 When I see `foo *', I *know* what it is. When I see `foop', I have to
287 Casting pointers to and from `void *' clutters the code and serves no
288 purpose (repeat after me: C is not C++). It is also unsafe, because
289 the cast suppresses type warnings.
291 Casting function values to void clutters the code and serves little
292 purpose (repeat after me: C is not Pascal).
294 Casting an argument to the type of the parameter clutters the code and
295 serves no purpose (repeat after me: ISO C is not K&R C).
297 Factor out common code
299 Do not gratuitiously duplicate code! Ken Stevens said it well, and
300 it's as relevant as ever:
302 Cut-and-Paste coding is by far the biggest problem that the current
303 Empire suffers from. This is how cut-and-paste coding happens. Joe
304 Shmuck decides that he wants to add a new function to the server.
305 So he goes hunting through the server to find some already existing
306 code which does something similar to what he wants to do. This is
307 good. You should always write new code to imitate old code. What
308 is bad is when Joe Shmuck decides to simply "copy" 200 lines of code
309 from the old function into his new function, and then just change a
310 couple of little things here and there to suit his needs. This
311 method, known as Cut-and-Paste coding is the fastest and easiest way
312 to code. However, it results in a server that is impossible to
313 maintain. What Joe _should_ have done, is "move" the 200 lines of
314 code into a new _third_ function which the first two both call.
315 This is called writing a "general solution" to handle both cases.
316 Most of my work in the Empire2 project consisted in cleaning up
317 after a bunch of Joe Shmucks.
325 FIXME sizes, printf formats
329 FIXME conditional compilation is a last resort
339 Check error conditions meticulously. The existing code is bad enough
340 at this, please don't make it worse.
342 FIXME what to do on error
350 Empire uses non-preemptive threads: a thread runs until it yields the
351 processor. This simplifies programming, but there are still traps to
352 avoid. Empire threads may yield the processor whenever they do I/O or
355 Be careful with static storage. Read-only data is fine. More than
356 one thread writing static data is problematic, but there are safe
357 uses. For instance, a static scratch buffer that is never used across
360 Thread stacks cannot grow. If you use too much automatic storage, you
361 can overrun the stack with disastrous consequences. FIXME elaborate
363 Yielding the processor invalidates *all* the game state you copied
364 into variables. You have to reread and possibly recheck. See below.
368 Game state consists of several sets of objects, e.g. sectors, ships,
369 bmaps, ... Each set is fully held in memory and backed by a file in
372 There are several ways to access an object in set FOO:
374 * You can get a pointer to the object (not a copy!) with getFOOp().
376 This is inherently dangerous, because it allows you to update the
377 object in memory without updating the disk file. It's used mostly
378 in update code, where it pays major performance dividends, but there
379 are uses in other places. Whether they are wise is debatable.
381 Obviously, the object can change when you let another thread run.
382 This is not an issue in update code, because it never yields.
384 * You can get a copy with getFOO() and write it back with putFOO().
386 Yielding the processor invalidates the copy. In particular, if you
387 yield the processor between get and put, and another thread changes
388 the game data, then put will clobber that change, possibly resulting
391 Therefore, you have to re-get after a yield, and repeat any checks
392 you already made. If you can afford to bail out when something
393 changed at all, use check_FOO_ok().
395 Putting updates both the object in memory and the disk file.
397 If you write a function that takes a pointer to an object, think
398 twice before you put it, and if you do, mention it in the function
401 * Bmaps have special access functions.
403 Each country has to bmaps: the working bmap and the true bmap.
404 Unfortunately, the bmap code called the former `bmap' and the latter
407 You update bmaps with map_set(). This doesn't write through to the
408 file; instead, it returns non-zero when the update changed anything.
409 In that case, you must write bmaps to disk with writemap() before
410 you yield. If you only updated the working bmap, then you can call
417 Commit related changes together, unrelated changes separately.
419 See chapter Change Logs in the GNU coding standards for guidelines on
420 log messages. Try http://www.gnu.org/prep/standards_40.html#SEC40 or
421 search the web for `gnu coding standards change logs'.
423 Don't change whitespace gratuitiously, say just because your editor
424 screws up tabs. Such changes make it much harder to figure out who
425 changed what and when.
428 Historical guidelines, superseded by the above
431 Remarks from Dave Pare:
433 And now, a few editorial remarks on my views on Good Techniques for
434 Modifying Existing Software:
436 My safari through the hot and bug-ridden jungle that Empire has become
437 has given me some new insights on the modification of large software
438 packages. My adventure has prompted me to propose some simple coding
439 guidelines for those dreaming of hacking on the code:
441 1) Be invisible. When you make a change, think not about marking your
442 place in history, or about showing off how much nicer your two-space
443 tabs are than those old, icky eight-space tabs that the stupid empire
444 hackers of old used, but think instead about the asethetics of the whole.
445 The resulting lines of code should flow smoothly from the beginning of the
446 procedure to the end. Empire is 60,000 lines of code. If you're the
447 general case, you're only changing fifty lines, so be aware of that.
449 2) Procedurize. If you find yourself in a triple-nested loop, or a five
450 hundred line procedure, perhaps it's because you need to split some of
451 that code off into a procedure. In general, if you've got more than two
452 levels of tabs (or indentations), or more than one or two major loops in
453 a procedure, something may well be amiss.
456 Sasha Mikheev on indentation:
458 The empire indentation style can be achived by using
459 indent -orig -i8 foo.c
461 or in old c-mode emacs (versions before 19.29):
463 (setq c-indent-level 8)
464 (setq c-continued-statement-offset 8)
465 (setq c-argdecl-indent 8)
466 (setq c-brace-offset -8)
467 (setq c-label-offset -8)
470 Further comments by Ken Stevens:
473 The only variables which should be global are constants. If you write
474 a routine which changes a global variable, then you will corrupt the
475 data when two different players run that routine at the same time.
483 Should be rewritten as:
489 AIX has different conventions for signed chars, and IRIX requires the
490 /* comments */ after #endif.
493 Cut-and-Paste coding is by far the biggest problem that the current
494 Empire suffers from. This is how cut-and-paste coding happens. Joe
495 Shmuck decides that he wants to add a new function to the server. So
496 he goes hunting through the server to find some already existing code
497 which does something similar to what he wants to do. This is good.
498 You should always write new code to imitate old code. What is bad is
499 when Joe Shmuck decides to simply "copy" 200 lines of code from the old
500 function into his new function, and then just change a couple of
501 little things here and there to suit his needs. This method, known as
502 Cut-and-Paste coding is the fastest and easiest way to code. However,
503 it results in a server that is impossible to maintain. What Joe
504 _should_ have done, is "move" the 200 lines of code into a new _third_
505 function which the first two both call. This is called writing a
506 "general solution" to handle both cases. Most of my work in the
507 Empire2 project consisted in cleaning up after a bonch of Joe Shmucks.
508 I took repeated code and "consolidated" it into general function
512 Just to add to Dave's "Be Invisible" motto, I'd like to give a little
513 example to illustrate some basic do's and don'ts for coding style:
515 The following function has bad style:
517 double att_combat_eff(com,own)
522 if(com->type==EF_SECTOR)
526 str=com->sct_dcp->d_ostr;
528 str=com->sct_dcp->d_dstr;eff=2.0+(str-2.0)*eff;
529 }else if(com->type==EF_SHIP&&com->own!=own)
530 eff=(1.0+com->shp_mcp->m_armor/100.0);
533 Here is the same function written with "good" style:
536 att_combat_eff(com, own)
543 if (com->type == EF_SECTOR) {
544 eff = com->eff / 100.0;
546 str = com->sct_dcp->d_ostr;
548 str = com->sct_dcp->d_dstr;
549 eff = 2.0 + (str - 2.0) * eff;
550 } else if (com->type == EF_SHIP && com->own != own)
551 eff = (1.0 + com->shp_mcp->m_armor / 100.0);
556 These are all the things I fixed when changing the bad to the good:
557 - Function names should always start a new line (so you can search for them)
558 - There should always be a space after a ","
559 - Function arguments should be indented 8 spaces
560 - There should always be a tab after a type declaration
561 - Opening function bracket should be on a line by itself
562 - Indentation should be 8 spaces
563 - There should always be a space on both sides of every operator
564 - There should always be a space after if, for, while, switch
565 - The opening bracket should be on the same line as the if
566 - There should always be a space on either side of a {
567 - There should always be a new line after a ;
568 - The closing function bracket should be on a line by itself