Some CSBwin (potentialy) bugs

[Ulmo]

I have read a large part of the CSBwin sources, trying to figure out was the "rules" of the game are. I found some things that looked between weird and false. Here are the indications to check, and maybe correct those things.

The File/Function/Line indications are taken from the CSBwin 9.5 sources.


1) There is a constant MAXLOAD, set to 2000. But you can exceed that load (not easily) when carrying many knigts' armors with the help of chest. I finally understood that asserts are not checked in release mode, so this probably does not matter.

2)
File : Character.cpp
Function : TAG01680a
Line : 2092
STRandom0_3();
Standing alone ?? And the following line uses an old (and potentially huge) value for D0W.

3)
File : CSBCode.cpp
Function : void SelectPaletteForLightLevel(void)
Lines : 5569 & 5571
The loop runs 5 times, but only 4 values are sorted. But it seems that it does not matter because the fifth value is always zero : (0..15) / 16 = 0.

4)
File : CSBCode.cpp
Function : NinetySecondUpdate
Line : 5634
"D5W = 1" seems to be misplaced : the light level is computed again only if a torch has burned out, and not if it has just lost a charge.

5)
File : Character
Function : _FeedCharacter
Line : 3127
Shouldn't the strength of a shield potion be decreased instead of incresed when the shield is already high ?

6)
File : Attack.cpp
Function : i16 TAG00f930()
Line : 461
D6 can be uninitialized if throwingdistance==0 (difficult but possible).
Maybe the line 413 should go to 421...
(You should trust you compiler ! the warning (cf Line 280) was right!)

7)
File : AsciiDump.cpp
Function : DumpAttacks
Lines : 1807-...
The "Time" values are printed as %02d, but claimed as "0x" values.
"Time" values are experience earned.
"Probability" values are used as a time when actions are disabled.



The following are bugs from the original game. But I think that they should be corrected, especially the one making Antifire/Antimagic unused. As CSBwin is claimed to be "The original FTL game", you could choose an option as for the expanded databases : a question like "do you want to play the game as it was released or as it was designed ?"

8)
File : Character.cpp
Function : TAG0183ea
Line : 2775
Do empty flasks really have a power ?

9)
File : Monster.cpp
Function : StealFromCharacter
Lines : 152-156
There is all the needed code to allow gigglers to steal in other places than the left hand, especially in the backpack, but the d.ubyte13126[0-7] array is filled with 0s.

10)
File : Character.cpp
Function : i16 TAG016426(CHARDESC *pChar,i16 attrNum,i16 P3)
Line : 1842
This is a real bug ! It is funny seeing how you reproduced it.
this function is used with :
* Vitality when you get poisoned by a monster (this is the way for me to be sure that this bug was present in ST DM, because I clearly remember that scorpions did 4 damage poison)
* Antifire when receiving a fireball, or being attacked by a black flame
* Antimagic when being attacked by LordChaos/Zytaz/Vexirk/Flying_Eye
* Vitality when being hurt, to avoid injuries ("ouches").

The bad thing is that Antimagic and Antifire have no use at all...

[Paul Stevens]

You have done some amazing work. I appreciate it
very much. Deciphering this code is not easy as anyone
who has tried it will testify. Thanks for the reports.

I will follow up on these individually as I check them out
myself and make changes or whatever.

1) MAXLOAD = 2000 (200 * 10)
Indeed. You are right. I had trouble with the loads going
negative and put in the limit as a debug aid. I have added
comments to indicate the purpose of the MAXLOAD tests
so as not to confuse the reader quite so easily.

[Paul Stevens]

2) STRandom0_3() standing alone.

You are certainly right....unfortunately we were saved from
disaster by the following limits test and so it never caused
enough of a problem to be noticed.

It is fixed.

[Paul Stevens]

3) SelectPaletteForLightLevel

I think it is properly translated from the original. It has the
effect (I think) of including the contribution of a fifth
torch if the party is carrying more than four. The fifth would
be chosen more-or-less at random after the four best
torches are located. Since it does no harm and is faithful
to the original, I propose to leave it alone.

Here is the original code...perhaps you see where I made
a mistake. These are the two loops: one to sort and the
second to add up the torches' brightness.

BTW...it can make a difference in the summation....those values
in the lighting table are as great as 99.

//01806c 41ee ffe6 LEA -26(A6),A0
//018070 2648 MOVE.L A0,A3
//018072 4244 CLR.W D4
//018074 6034 BRA $+54 (=0x0180aa)
//018076 3a3c 0007 MOVE.W #7,D5
//01807a 9a44 SUB.W D4,D5
//01807c 3004 MOVE.W D4,D0
//01807e e380 ASL.L #1,D0
//018080 41ee ffe8 LEA -24(A6),A0
//018084 d0c0 ADD.W D0,A0
//018086 41d0 LEA (A0),A0
//018088 2448 MOVE.L A0,A2
//01808a 6012 BRA $+20 (=0x01809e)
//01808c 3012 MOVE.W (A2),D0
//01808e b053 CMP.W (A3),D0
//018090 6f0a BLE $+12 (=0x01809c)
//018092 3d52 fff6 MOVE.W (A2),-10(A6)
//018096 3493 MOVE.W (A3),(A2)
//018098 36ae fff6 MOVE.W -10(A6),(A3)
//01809c 548a ADDQ.L #2,A2
//01809e 3005 MOVE.W D5,D0
//0180a0 5345 SUBQ.W #1,D5
//0180a2 4a40 TST.W D0
//0180a4 66e6 BNE $-24 (=0x01808c)
//0180a6 548b ADDQ.L #2,A3
//0180a8 5244 ADDQ.W #1,D4
//0180aa 0c44 0004 CMP.W #4,D4
//0180ae 66c6 BNE $-56 (=0x018076)
//0180b0 3c3c 0006 MOVE.W #6,D6
//0180b4 383c 0005 MOVE.W #5,D4
//0180b8 4247 CLR.W D7
//0180ba 41ee ffe6 LEA -26(A6),A0
//0180be 2648 MOVE.L A0,A3
//0180c0 6028 BRA $+42 (=0x0180ea)
//0180c2 3013 MOVE.W (A3),D0
//0180c4 6722 BEQ $+36 (=0x0180e8)
//0180c6 3013 MOVE.W (A3),D0
//0180c8 e380 ASL.L #1,D0
//0180ca 41ec fbce LEA -1074(A4),A0
//0180ce d0c0 ADD.W D0,A0
//0180d0 3010 MOVE.W (A0),D0
//0180d2 ed60 ASL.W D6,D0
//0180d4 ec48 LSR.W #6,D0
//0180d6 de40 ADD.W D0,D7
//0180d8 3006 MOVE.W D6,D0
//0180da 5340 SUBQ.W #1,D0
//0180dc 3f00 MOVE.W D0,-(A7)
//0180de 4267 CLR.W -(A7)
//0180e0 4ead 00d2 JSR 210(A5) (=0x000ff8)
//0180e4 588f ADDQ.L #4,A7
//0180e6 3c00 MOVE.W D0,D6
//0180e8 548b ADDQ.L #2,A3
//0180ea 3004 MOVE.W D4,D0
//0180ec 5344 SUBQ.W #1,D4
//0180ee 4a40 TST.W D0
//0180f0 66d0 BNE $-46 (=0x0180c2)

[Paul Stevens]

4) Recomputing palette in NinetySecondUpdate...

Oooooops. My " if (D3W==0) " jumped one instruction too far.

Fixed.

[Paul Stevens]

5) Ya potion versus Shield Strength.

I really goofed here. My fingers must have repeated the
previous operation without my looking at the actual
code. Or I saw "ADD" where it said "MOVE".

It should say:

additive = potion strength;
if (Shield Strength > 50) additive /= 4;
Shield Strength += additive;

Ulmo: Would you like the original code so that you
don't have to guess where I made errors?

[Paul Stevens]

6) D6 uninitialized if throwingDistance == 0.

I do trust the compiler. But it cannot know everything.
And I do not know everything. That is why I put
an ASSERT to catch the case. The ASSERTion has never
been triggered.

I think the Atari code left this variable uninitialized. I am
perfectly willing to initialize it for the case where
throwingDistance==0. But I don't know what to
initialize it to! I will entertain suggestions.

[Paul Stevens]

7) Attack Dump

I fixed the decimal versus hex inconsistency.

I am not sure I understand the rest of your note.
It seems to say that the column now labeled "Time"
should be labeled "Experience". Is that correct?
And that there should be no column labeled "Time".

[Paul Stevens]

8) Strength of Empty Flask.

I added:
&& (otD6 != obj_EmptyFlask)

[Ulmo]

> I do trust the compiler. But it cannot know everything.
> And I do not know everything. That is why I put
> an ASSERT to catch the case. The ASSERTion has never
> been triggered.

I probably forgot to say something : I am really sure that you did a very good job, that you know very well what you are doing, that you are an experimented programmer. Moreover I would be completely incapable to write such a project, so my tone is certainly not the "you should have done like this if you were serious" one.
So please assume I had to put a smiley after the "you should trust the compiler" : here it is :-)
For sure you cant know everything, that is why I am trying to bring my contribution.

> Ulmo: Would you like the original code so that you
> don't have to guess where I made errors?

Sorry, I'm not capable to read ASM code. The only ASM I learned was the Apple II 's 6502 one, that is way too old.




3) SelectPaletteForLightLevel
> Since it does no harm and is faithful
> to the original, I propose to leave it alone.

I really agree with you. As it looked weird, I wanted to check it was correct.

> BTW...it can make a difference in the summation....those values
> in the lighting table are as great as 99.

Yes, you're right : I messed the light value with the remaining charges...

6) D6 uninitialized if throwingDistance == 0.

> I think the Atari code left this variable uninitialized. I am
> perfectly willing to initialize it for the case where
> throwingDistance==0. But I don't know what to
> initialize it to! I will entertain suggestions.

You could track D6 register's values in the ASM program to have an idea of the "right" value :-)
More seriously, it looks very possible that the "if" statement at line 374 should end at line 411 instead of line 420. In that case, monsters keep their armor even if hit by "throwingdistance==0".
So my opinion is that this should be the correction to do.

7) Attack Dump

> I am not sure I understand the rest of your note.
> It seems to say that the column now labeled "Time"
> should be labeled "Experience". Is that correct?
> And that there should be no column labeled "Time".

What I meant was that the column currently labeled "time" shows values that are use to increase experience; and that the "probability" labeled values are used to disable the right hand action during this amount of time.
So my claim is that "time" should become "experience", and that "probability" should become "time".
After (re)checking, I also claim that "Strength required" is used to try to hit (TAG00f930 function, parameter P7). The smaller, the easier.
I agree with other labels.

[Paul Stevens]

6) D6 uninitialized if throwingDistance == 0.

Here is what I am going to do. That function is
called from only one place. D6W is well-defined
at that place. So in the original Atari code you
could predict what would happen even if it was
quite wrong!

I am going to pass that 'well-defined' value of D6W
to the inner function so that whatever happens it
will happen in the same way that it did on the Atari
version.

I will make it 'right' if someone can convince me that
he knows what IS right.

[Ulmo]

I think that 'Being the same than on ST' looks like a good definition af 'right'.

Another definition could be 'being what FTL wanted to do', by this is much more difficult to deal with.
Nonetheless I still think that some points like 9) or 10) should be discussed here and maybe changed.

[Paul Stevens]

Indeed.

There is no way to know what was intended.

They may have decided that Gigglers were simply too
obnoxious when they could steal from you backpack
or whatever. Or they may have forgotten to create
the table. I have had no feedback whatsoever from
anyone else about this. There are eight entries in the
table. Do you suppose that all were to be used?
1 left hand
2 right hand
3 feet
4 legs
5 torso
6 head
7 necklace
8-11 quiver (four places)
12-13 pouch (two places)
14 backback - position chosen at random

For each entry in the table there is a one-in-eight chance
that the entry will be selected. You could put the backpack
in the table twice and then it would have a one-in-four
chance. Do we want Gigglers stealing your helmet?

Or, since we are defining this, we could make the table
larger.

So........? ? What to do? I can make it a designer's choice
if we wish to go that route.

[Ulmo]

In some clones, gigglers can steal everywhere, and I do not find it *so* annoying. And I remember that when I first played DM, I've found surprisingly easy to make gigglers harmless.
But I think it is very ridiculous when someone steals my shoes or my armor...
Hands, pouch and backpack seems 'normal' places to me. Maybe quiver's first place too.

> I have had no feedback whatsoever from anyone else about this.
I think it is difficult to notice. Until reading your code, I had never imagined that gigglers could steal somewhere else. I mean in the DM world.

> I can make it a designer's choice if we wish to go that route.
Does it mean that you get a designer label and make a non-programmer's choice ? Or that you leave this decision to any dungeon's designer ?
If, as I understood, this array is read from dungeon.dat, CSBuild sounds a good tool to change this.

[Paul Stevens]

I got news for you. It ain't initialized anywhere
that I can find. I clear the entire data segment
at the start of execution and the array is never
written to again.

CSBuild will allow you to specify these eight values.
They will default to zeroes as in the standard game.

[Paul Stevens]

This is a real bug ! It is funny seeing how you reproduced it.
The bad thing is that Antimagic and Antifire have no use at all...
============================================
I agree that they could not have meant this! On the other
hand, it has never been tested! In the Atari version they would
always have gotten a zero; no other value has been tested.

If I make the change and post a CSBwin.exe will you try it
out to see if it does something reasonable?

[Ulmo]

> CSBuild will allow you to specify these eight values.
Fine.

> If I make the change and post a CSBwin.exe will you try it
> out to see if it does something reasonable?

Of course.
In fact, I already tested it a little (thanks to the distributed sources). Fireball do less damage, but keep dangerous. As a byside, storms (OHKATHRA spells) get more dangerous, as you do not resist them.
I didn't look closer to other changes (injuries resistance, antimagic). I will, and then I will report.

[Paul Stevens]

Anyone who wants to try out the Anti-Magic and -Fire
can get the CSBwin.exe at
http://bahamas.globat.com/~dianneandpaul.net

it is named AntiMagicFire.rar

Let me know if it seems ok to release.

[PicturesInTheDark]

What exactly should be different within this version, so that we know what we are testing it against?

Regards, PitD

[Paul Stevens]

See item 10) in the original note in this thread.
Other than that, I don't know. I just changed
which byte was used; I don't know what it does.

PAul