X-Git-Url: https://jxself.org/git/?p=open-adventure.git;a=blobdiff_plain;f=notes.adoc;h=1baf354d694a667a9c81a3049765ad15048d1587;hp=1c06c809b8990fb3dae6cd31d65e952dcdc23c39;hb=236abc8cab5a6f8d0b1d1921800a8645dcda98a2;hpb=1b167e5e72e0b4ea4b991293abaf9b8ff1eac599 diff --git a/notes.adoc b/notes.adoc index 1c06c80..1baf354 100644 --- a/notes.adoc +++ b/notes.adoc @@ -9,7 +9,9 @@ separate link:history.html[history] describing how it came to us. The principal maintainers of this code are Eric S. Raymond and Jason Ninneman. Eric received Don Woods's encouragement to update and ship -the game; Jason signed on early in the process to help. +the game; Jason signed on early in the process to help. The assistance +of Peje Nilsson in restructuring some particularly grotty gotos is +gratefully acknowledged. Petr Voropaev contributed fuzz testing. == Nomenclature == @@ -60,14 +62,19 @@ a "seed" command) will replay reliably, including random events. The adventure.text file is no longer required at runtime. Instead, it is compiled at build time to a source module containing C structures, -which is then linked to the advent binary. +which is then linked to the advent binary. There is an adventure.yaml file +as well; this is also compiled to C code, and will eventually replace +adventure.text altogether. -The game-save format has changed. This was done to simplify -FORTRAN-derived code that formerly implemented these functions; -without C's fread(3)/fwrite() and structs it was necessarily pretty -ugly by modern standards. Encryption and checksumming have been -discarded - it's pointless to try tamper-proofing saves when everyone -has the source code. +The game-save format has changed. This was done to simplify the +FORTRAN-derived code that formerly implemented the save/restore +functions; without C's fread(3)/fwrite() and structs it was +necessarily pretty ugly by modern standards. Encryption and +checksumming have been discarded - it's pointless to try +tamper-proofing saves when everyone has the source code. + +A -r command-line been added. When it is given (with a file path +argument) it is functionally equivalent to a RESTORE command. == Translation == @@ -77,13 +84,15 @@ ugly and quite unreadable. Jason Ninneman and I have moved it to what is almost, but not quite, idiomatic modern C. We refactored the right way, checking correctness -against a comprehesive test suite that we built first and verified with -coverage tools. This is what you are running when you do "make check". +against a comprehensive test suite that we built first and verified +with coverage tools (we now have over 90% coverage, with the remaining +confined to exception cases that are very difficult to reach). This is +what you are running when you do "make check". -This move entailed some structural changes. The most important was -the refactoring of 354 gotos into if/loop/break structures. We -also abolished almost all shared globals; the main one left is a -struct holding the game's saveable/restorable state. +The move to modern C entailed some structural changes. The most +important was the refactoring of over 350 gotos into if/loop/break +structures. We also abolished almost all shared globals; the main one +left is a struct holding the game's saveable/restorable state. The original code was greatly complicated by a kind of bit-packing that was performed because the FORTRAN it was written in had no string @@ -99,7 +108,12 @@ in favor of proper C strings. C strings may be a weak and leaky abstraction, but this is one of the rare cases in which they are an obvious improvement over what they're displacing... -The code falls a short of being fully modern C in the following +We have also conducted extensive fuzz testing on the game using +afl (American Fuzzy Lop). We've found and fixed some crashers in +our new code (which occasionally uses malloc(3)) but none as yet +in Don's old code (which didn't). + +The code falls short of being fully modern C in the following ways: * We have not attempted to translate the old code to pointer-based @@ -108,10 +122,9 @@ ways: and the choice to refrain will make forward translation into future languages easier. -* There are 20 gotos left that resist restructuring; all but one of - these are in the principal command interpreter function implementing - its state machine. A 21st, a two-level loop breakout, is not reducible - even in principle. +* There are a few gotos left that resist restructuring; all are in the + principal command interpreter function implementing its state + machine. * Linked lists (for objects at a location) are implemented using an array of link indices. This is a surviving FORTRANism that is quite unlike @@ -119,7 +132,13 @@ ways: to fix it because doing so would (a) be quite difficult, and (b) compromise forward-portability to other languages. -* The code still has an unfortunately high density of magic numbers - in - particular, numeric object and room IDs. +* Much of the code still uses FORTRAN-style uppercase names. + +* The code still assumes one-origin array indexing. Thus, arrays are + a cell larger than they strictly need to be and cell 0 is unused. + +* The code is still mostly typeless, slinging around machine longs + like a FORTRAN or BCPL program. Some (incomplete) effort has been made + to introduce semantic types. // end