]> git.pond.sub.org Git - empserver/blobdiff - doc/coding
doc/contributing: New file
[empserver] / doc / coding
index 179a96c203dc4711831f3f92bc7d2af7d8d04309..c32b39268a0383df5426f8bf1832ed3ab951c140 100644 (file)
@@ -44,6 +44,8 @@ These guidelines don't attempt to be exhaustive.  More complete
 guidelines that are mostly compatible with Empire can be found at
 http://www.jetcafe.org/~jim/c-style.html
 
+See also doc/contributing.
+
 
 Source tree organization
 ------------------------
@@ -78,6 +80,12 @@ you set the style for the current buffer.  Set variable
 `c-default-style' to "stroustrup" to switch to this style for all
 buffers.
 
+Avoid gratuitous space change
+
+Don't change whitespace gratuitiously, say just because your editor
+screws up tabs.  Such changes make it much harder to figure out who
+changed what and when.
+
 Tab character use
 
 Whether you use tab characters or not doesn't really matter that much,
@@ -288,6 +296,10 @@ Do not use increment operators to set a variable to logical true!  It
 obfuscates the purpose, and narrow variables could even overflow.
 Just assign 1.  A lot of cleanup remains to be done there.
 
+Null pointer constant
+
+Please use NULL for clarity, not just 0.
+
 Type casts
 
 Casting pointers to and from `void *' clutters the code and serves no
@@ -345,6 +357,14 @@ at this, please don't make it worse.
 
 FIXME what to do on error
 
+Sanity checking
+
+In many places, the code checks for conditions that should not happen,
+and then tries to recover.  This is sound defensive programming.
+Please use CANT_HAPPEN() and CANT_REACH() for this purpose, because
+they log the error condition, and optionally abort the program, or
+write out a core dump.  This is called an "oops".
+
 Buffer overruns
 
 FIXME
@@ -394,20 +414,29 @@ There are several ways to access an object in set FOO:
 
 * You can get a copy with getFOO() and write it back with putFOO().
 
-  Yielding the processor invalidates the copy.  In particular, if you
-  yield the processor between get and put, and another thread changes
-  the game data, then put will clobber that change, possibly resulting
-  in a corrupted game.
+  Putting updates both the object in memory and the disk file.
 
-  Therefore, you have to re-get after a yield, and repeat any checks
-  you already made.  If you can afford to bail out when something
-  changed at all, use check_FOO_ok().
+  Any change to the object invalidates the copy.  Putting such an
+  invalid copy will clobber the change(s) that invalidated it,
+  possibly resulting in corrupted game state.  The code oopses on such
+  puts, but it can't repair the damage.
 
-  Putting updates both the object in memory and the disk file.
+  There are two common ways to invalidate a copy: calling a function
+  that updates the object you copied (unless it does that through your
+  copy), and yielding the processor, which lets other threads update
+  the object you copied.
+
+  Therefore, you have to reget after a possible invalidation, and deal
+  with changes.  In particular, if you checked whether the object is
+  suitable for a task, you need to check again after regetting it.  If
+  you can afford to bail out when something changed, use
+  check_FOO_ok().
 
-  If you write a function that takes a pointer to an object, think
-  twice before you put it, and if you do, mention it in the function
-  comment.
+  Function comments should state what objects the function can update.
+  Unfortunately, they generally don't.
+
+  It's best to keep puts close to gets, both in runtime and in the
+  source code.
 
 * Bmaps have special access functions.
 
@@ -421,19 +450,25 @@ There are several ways to access an object in set FOO:
   you yield.  If you only updated the working bmap, then you can call
   writebmap() instead.
 
+Reading player input
 
-CVS
----
+Reading player input can fail, and you must check for that.
+Neglecting it can break the interrupt feature for players (normally
+Ctrl-C), and produce extra prompts that could confuse clients.  Even
+worse is neglecting it in a loop that terminates only when input is
+read successfully.
 
-Commit related changes together, unrelated changes separately.
+When reading input fails, you should normally abort the command with
+status RET_SYN.  There are exceptions, e.g. when aborting a pinpoint
+bombing run over the target.
 
-See chapter Change Logs in the GNU coding standards for guidelines on
-log messages.  Try http://www.gnu.org/prep/standards_40.html#SEC40 or
-search the web for `gnu coding standards change logs'.
+Some functions to read player input return a special error value you
+can check, e.g. recvclient(), prmptrd() and uprmptrd() return a
+negative value, getstring() and getstarg() return NULL.
 
-Don't change whitespace gratuitiously, say just because your editor
-screws up tabs.  Such changes make it much harder to figure out who
-changed what and when.
+Others return the same error value for failed read and for successful
+read of input that is invalid.  Then you need to check
+player->aborted; if it is set after a read, the read failed.
 
 \f
 Historical guidelines, superseded by the above
@@ -466,7 +501,7 @@ a procedure, something may well be amiss.
 
 Sasha Mikheev on indentation:
 
-The empire indentation style can be achived by using 
+The empire indentation style can be achived by using
 indent -orig -i8 foo.c
 
 or in old c-mode emacs (versions before 19.29):
@@ -577,5 +612,3 @@ These are all the things I fixed when changing the bad to the good:
 - There should always be a space on either side of a {
 - There should always be a new line after a ;
 - The closing function bracket should be on a line by itself
-
-