[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Re-engineered: Wizardry III, Legacy of Llylgamyn
In this post I will describe the software defects ("bugs") I've
discovered in the code. These are not bugs that I've introduced into
the code, but bugs that the original authors wrote.
For example, if the original code misspelled a word in a message (such
as "INDENTIFY"), then the code that I re-engineered also had to
misspell the word in the source code so that it would match the
original code. As another example, if the original code used the
wrong subscript in an array, then the code that I wrote had to also
use the wrong subscript in the array.
Bug #1
------
During the transfer of characters to LOL, the message states that the
characters must be "IN CASTLE" and "LIVE". The code however, only
checks "STATUS=OK":
690 7 24:D 1 PROCEDURE REMOVCHR; (* P050118 *)
717 7 24:4 126 WRITELN( 'NOTE - CHARS MUST BE LIVE & IN
CASTLE');
706 7 24:4 72 IF PLAYREC2.STATUS = OK THEN
Bug #2
------
During the transfer of characters to Legacy of LLylgamyn, it is
possible to get caught in an infinite loop.
The following code tries to write a new value to BL5BUFF[ 12] on the
FROM disk. If the disk is write protected (as the instructions in the
manual suggest you do), then the value on the diskette is now
different from the value in BL5BUFF[ 12] and FRMRNDID (From Disk
Random ID).
674 7 23:D 1 PROCEDURE TRANSFER; (* P050117 *)
788 7 23:1 70 RDSCNTOC( 'INSERT SCENARIO WITH CHAR(S)
TO BE MOVED');
789 7 23:1 115 UNITREAD( DRIVE1, BL5BUFF,
SIZEOF( BL5BUFF), SERIALBL, 0);
790 7 23:1 127 MPD3 := BL5BUFF[ 10];
791 7 23:1 135 MOVELEFT( BL5BUFF, SERIALFR, 7);
792 7 23:1 146 BL5BUFF[ 12] := CHR( (ORD( BL5BUFF[ 12])
+ RANDNUM) MOD 256);
793 7 23:1 162 UNITWRITE( DRIVE1, BL5BUFF,
SIZEOF( BL5BUFF), SERIALBL, 0);
794 7 23:1 174 FRMRNDID := BL5BUFF[ 12];
795 7 23:1 182 REMOVCHR;
The following code tries to write the status of "LOST" into the
players record we are transferring FROM. The I/O of course will
fail, but no checking is performed. That is actually a good thing,
but...
690 7 24:D 1 PROCEDURE REMOVCHR; (* P050118 *)
738 7 24:1 406 PLAYREC1 := PLAYREC2;
739 7 24:1 414 PLAYREC1.STATUS := LOST;
740 7 24:1 418 WRCHARAC( PLAYREC1, PLAYINDX);
741 7 24:1 427 WRICACHE
...if the TO disk is full, or the player name already exists on the TO
disk, then...
674 7 23:D 1 PROCEDURE TRANSFER; (* P050117 *)
809 7 23:5 334 IF (PLAYREC1.NAME =
PLAYREC2.NAME) AND
810 7 23:5 340 (PLAYREC1.STATUS <> LOST)
THEN
811 7 23:6 347 PLAYINDX := -1
818 7 23:1 385 IF (PLAYINDX = - 1) OR
819 7 23:1 391 (PLAYINDX = SCNTOC.RECPERDK[ ZCHAR])
OR
820 7 23:1 404 (COPY( SCNTOC.GAMENAME, 1, 10) <>
'THE LEGACY') THEN
821 7 23:2 437 TRANBAD
...the following code now tries to put the character back onto the
FROM disk, but since FRMRNDID was never written to the disk, we are
stuck in the following REPEAT-UNTIL loop:
745 7 25:D 1 PROCEDURE TRANBAD; (* P050119 *)
749 7 25:2 0 REPEAT
750 7 25:3 0 IF PLAYINDX = -1 THEN
751 7 25:4 9 RDSCNTOC( 'DUPLICATE NAME - PUT
IN SOURCE')
759 7 25:2 190 UNTIL (SCNTOC.GAMENAME = FRMGAMNM)
AND (FRMRNDID = BL5BUFF[ 12]);
Bug #3
------
A wrong message is displayed when "M"aking a Scenario Disk.
Here is the code at the beginning of MAKESCEN:
868 8 1:D 1 SEGMENT PROCEDURE MAKESCEN; (* P050201 *)
1119 8 1:1 0 REPEAT
1120 8 1:2 0 WRITE( CHR( HOMECLR));
1121 8 1:2 8 WRITE( 'DO YOU HAVE 1) OR 2) DRIVES ?');
1122 8 1:2 47 READ( INCHAR);
1123 8 1:1 55 UNTIL (INCHAR = '1') OR (INCHAR = '2');
1124 8 1:1 64 DRIVECNT := ORD( INCHAR) - ORD( '0');
The following code is testing the wrong variable, BASE04, instead of
DRIVECNT. As it turns out, BASE04 is always 1 here:
1129 8 1:1 117 IF BASE04 = 1 THEN
1130 8 1:2 122 BEGIN
1131 8 1:3 122 GOTOXY( 0, 3);
1132 8 1:3 127 WRITELN( 'INSERT AND REMOVE DISKS WHEN
PROMPTED.');
1133 8 1:3 181 WRITELN;
1134 8 1:3 187 WRITELN( 'REMEMBER : "BLANK" = YOUR
BLANK DISK.');
1135 8 1:3 241 WRITELN( ' "MASTER" = MASTER
SCENARIO.');
1136 8 1:2 295 END;
Bug #4
------
While "C"hanging a name of a character, a different error message is
displayed than the one you might expect.
The code in GETNAME() passes by reference a string defined as 40
characters into a routine that defines the parameter as 80
characters. The called routine updates this string and can therefore
overwrite (or clobber) a variable that follows it.
PASSWD, TONAME, and FROMNAME are all 40 characters in length:
441 7 17:D 1 PROCEDURE CHGNAMES; (* P050111 *)
444 7 17:D 1 PASSWD : STRING[ 40];
445 7 17:D 22 TONAME : STRING[ 40];
446 7 17:D 43 FROMNAME : STRING[ 40];
CHARNAME is the default value of 80 characters in length:
453 7 18:D 1 PROCEDURE GETNAME( VAR CHARNAME : STRING;
(* P050112 *)
GETNAME passes TONAME by reference (a 40 character name):
488 7 17:2 185 GETNAME( FROMNAME, 10, 'FROM NAME >');
489 7 17:2 204 GETNAME( TONAME, 12, ' TO NAME >');
To demonstrate the bug:
1. Start the game and select "U"tilities and "C"hange Name
2. Enter a "From>" name that is valid
3. Enter a "To>" name that is > 42 characters (will be off the
screen to the right)
4. Enter a valid "To>" name.
5. Result is: NAME NOT FOUND
That is a different error than if you type a "To>" name < 40
characters.
Note: This is at least one reason why the code uses the "$R-"
compiler option, otherwise it would not even compile.
Bug #5
------
SERIAL is defined as "STRING[7]". Sometimes it is referenced in the
code as 7 bytes, and sometimes as 8 bytes:
139 1 1:D 6 SERIAL : STRING[ 7];
1143 9 1:D 1 SEGMENT PROCEDURE OPTIONS; (* P050301 *)
1165 9 1:1 466 MOVELEFT( IOBUFF, SERIAL, 8);
1187 1 3:D 3 FUNCTION CHKCOPY : BOOLEAN; (* P050003 *)
1194 1 3:1 11 MOVELEFT( BUFFER, SERIAL, 7);
Bug #6
------
Variable RANDNUM is referenced on the right side of an assignment
before it has been initialized. (Probably not really a big deal?)
When the following code is first executed, RANDNUM has not been
assigned a value yet:
227 7 4:D 1 PROCEDURE GETKEY; (* P050104 *)
233 7 4:1 21 RANDNUM := (RANDNUM + ORD( INCHAR)) MOD
1027
Bug #7
------
IORESULT is not properly used.