killap said:
Haenlomal said:
At any rate, I believe that I have isolated the cause of the bug, and will submit a fix for it shortly.
Will we be seeing an sfall update this week then? No rush, but I don't want to make a release when a day later an sfall update comes out.
That depends on a couple of things:
1) Timeslip's availability to build a new version of sfall (I understand he's quite busy with RL stuff right now).
2) The soundness of my fix -- especially with asm, there's always the possibility of unintended side effect.
With point 2) in mind, I'll outline what I feel is the bug, and my fix for it. Just in case I missed something, maybe the rest of you will spot it.
As Timeslip alluded to in an earlier post, the error is located in the function op_wield_obj_critter. For those of you with Ollydbg or some other similar software, the function starts at offset 0x0045681C and ends at offset 0x004569CE. This is a somewhat long function, but fortunately, we can ignore most of it for our purposes.
There are two relevant parts of the code. This is the first part:
Code:
0045690A . E8 ED110200 CALL FALLOUT2._item_get_type
0045690F . 85C0 TEST EAX,EAX
00456911 . 75 0E JNZ SHORT FALLOUT2.00456921
00456913 . A1 B8106600 MOV EAX,DWORD PTR DS:[6610B8]
00456918 . E8 EBB20100 CALL FALLOUT2._inven_worn
0045691D . 894424 14 MOV DWORD PTR SS:[ESP+14],EAX
00456921 > BD 01000000 MOV EBP,1
00456926 . 8B0424 MOV EAX,DWORD PTR SS:[ESP]
00456929 . 896C24 18 MOV DWORD PTR SS:[ESP+18],EBP
Here's what happening here: op_wield_obj_critter calls function item_get_type to determine the exact type of item being wielded. In offset 0x0045690F, it tests if the function return is 0, which corresponds to item_type_armor (see define.h in the Fallout 2 Mapper source scripts). If not, it jumps ahead to offset 0x00456921, where register ebp is hardcoded to take on the value of 1, and this value is then stored in the stack ss:[esp+18]. If yes, then the engine makes the player wear the armor before setting ebp to 1 and storing it in ss:[esp+18]. The important thing to note here is that no matter what type of item is being wielded, ss:[esp+18] will end up with a value of 1.
The second relevant part of the code occurs a few lines down, when the engine finally touches ss:[esp+18] again:
Code:
00456962 . 3B05 B8106600 CMP EAX,DWORD PTR DS:[6610B8]
00456968 . 75 5B JNZ SHORT FALLOUT2.004569C5
0045696A . 8B5C24 18 MOV EBX,DWORD PTR SS:[ESP+18]
0045696E . B9 01000000 MOV ECX,1
00456973 . 85DB TEST EBX,EBX
00456975 . 74 0D JE SHORT FALLOUT2.00456984
00456977 . 8B5C24 0C MOV EBX,DWORD PTR SS:[ESP+C]
0045697B . 8B5424 14 MOV EDX,DWORD PTR SS:[ESP+14]
0045697F . E8 74AC0100 CALL FALLOUT2._adjust_ac
Here's what happening: first, the engine checks if the critter wielding the item is dude_obj (the player). If no, it skips ahead to a point outside the quoted code segment. If yes, the value of ss:[esp+18] is moved to register ebx, and then in offset 0x00456973, ebx is tested to see if it contains the value of 0. If yes, it skips on ahead out of the quoted code segment. Otherwise, it calls the function adjust_ac() to adjust the player's armor related stats.
The problem, of course, is that ebx will
NEVER be zero. In fact, after assigning the value of ebp to ss:[esp+18] in the first code segment, ss:[esp+18] is not touched again in any shape or form until the second part of the code segment. So this means that ebx will always be 1. This also means that if the critter is the player, adjust_ac() will be called regardless of whether or not the item wielded is a piece of armor. Taking a quick peak at adjust_ac() seems to indicate that it doesn't check its arguments, so it just grabs whatever happens to be in the relevant memory locations, leading to the bug described by Darek.
My fix deals exclusively with the first code segment. I injected some asm code at offset 0x0045690F:
Code:
xor ebp,ebp; // Set ebp=0 to skip adjust_ac() processing (assume not armor)
test eax,eax; // Did item_get_type() return item_type_armor?
jnz exit; // Skip ahead if no
mov eax,dword ptr ds:[0x6610b8];
call inven_worn; // Otherwise, wear armor
mov dword ptr ss:[esp+0x14],eax; // Store pointer to armor worn (for adjust_ac())
inc ebp; // Toggle flag to process adjust_ac() later in function
exit:
jmp WieldObjCritterFixEnd;
Note: WieldObjCritterFixEnd jumps the code back to offset 0x00456926.
Here, I've set the ebp flag to be zero by default. If get_item_type() returns item_type_armor, then ebp is incremented (i.e. ebp=1). Otherwise, it remains zero, and shouldn't trigger adjust_ac() further on down the road.
Questions and comments are welcomed.
Cheers,
-- The Haen.
Edit: quoted code segments weren't displaying function names properly