New Reno boxing bonus stats - Feature, or just a bug?

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.
 
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
 
NMA - modders fighting over fixing a bug. Hehe.

You should test that solution in-game, though I have a feeling it would work.

And, by the way, why do you refer to Timeslip as "he"? I remember some post from Timeslip, saying that "she" is actually a woman. But I'm not sure that post was by Timeslip, and I apologise if I'm wrong.
 
Blackened said:
And, by the way, why do you refer to Timeslip as "he"? I remember some post from Timeslip, saying that "she" is actually a woman. But I'm not sure that post was by Timeslip, and I apologise if I'm wrong.

Hehe - the mystery continues. I think we should set Detective Darek on the case to find out for us. If anyone can, Darek can.

But more seriously and on-topic - great work guys/girls. This is an important fix for FO2 and is a long time coming. It will have quite an impact on gameplay too .... ;) :D
 
Hmm, I think I miss a whole set of posts...

And yes, to me this fix is bigger than the "too many items" bug fix. So big that I'll delay the playthroughts I plan for when this bugfix comes out in some kind of patch (or much later, since my RL is a bitch to my Fallout side). Great work.
 
Blackened said:
And, by the way, why do you refer to Timeslip as "he"? I remember some post from Timeslip, saying that "she" is actually a woman. But I'm not sure that post was by Timeslip, and I apologise if I'm wrong.

I'm aware that different people in the forums have at various times referred to Timeslip as both "he" and "she". As far as I am aware, the only comment that Timeslip himself has made on the issue is that he has never confirmed one way or the other (or something to that effect).

Given that a gender neutral pronoun doesn't exist in English (yes, there's "it", but somehow I don't think that's going to go over well :P), and given that there has been no confirmation, I've decided to stick with the male pronoun unless information that indicates otherwise becomes known.

-- The Haen.
 
Josan12 said:
Hehe - the mystery continues. I think we should set Detective Darek on the case to find out for us. If anyone can, Darek can.
:look: :look: :look:

half-man-half-woman11.jpg
 
Darek said:
The weapon will get equipped to whatever hand I happen to have active. The bonus is applied on both hands and it's permanent.
Then I was mistaken. Haenlomal has obviously spent far more time than time looking at it than me, so no surprise there.

killap said:
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.
No.

Haenlomal said:
(yes, there's "it", but somehow I don't think that's going to go over well :P)
:rofl:

Haenlomal said:
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.
Your reasoning looks good, but I'm not certain of your fix. It may well just be me making more mistakes from trying to speed read the asm though...

Now that I've actually bothered to read the function description in the mapper manual, it says that whatever you equip always ends up in the hand, without mentioning anything about armour. I didn't see anything in the function to do anything other than weild the item, so presumably calling wield_obj_critter with a piece of armour causes the critter to hold the armour rather than wear it. In that case, the ac bonus shouldn't be applied ever, because that function never actually causes you to wear the armour. (And the inven_worn bit isn't needed either, since it just stores away the old piece of armour for when adjust_ac is called.)

Unless the function description is wrong, all that's required to fix the bug is the line 'BlockCall(0x45697F);'.
 
Dforge

I always thought that this is a bug, the bonus resistance doesn't make any sense. Also, losing plasma resistance sucks and possibly outweights the bonus because the only thing that can REALLY threaten you is plasma damage.

Will there be any quick fix for RP 2.1 for this issue? A script that removes the added bonus from the player would be a great temporary fix until the proper engine issue is addressed and
 
Josan12 said:
Hehe - the mystery continues. I think we should set Detective Darek on the case to find out for us. If anyone can, Darek can.
I just asked the magic eight-ball if Timeslip is a woman.
"Without a doubt" was the reply. So there you have it, case closed. :P

Timeslip said:
Now that I've actually bothered to read the function description in the mapper manual, it says that whatever you equip always ends up in the hand, without mentioning anything about armour. I didn't see anything in the function to do anything other than weild the item, so presumably calling wield_obj_critter with a piece of armour causes the critter to hold the armour rather than wear it. In that case, the ac bonus shouldn't be applied ever, because that function never actually causes you to wear the armour. (And the inven_worn bit isn't needed either, since it just stores away the old piece of armour for when adjust_ac is called.)

Unless the function description is wrong, all that's required to fix the bug is the line 'BlockCall(0x45697F);'.
I'm not sure if I understand what you mean correctly, but calling wield_obj_critter with a piece of armor will make you wear it in the armor slot, and not only "holding it" in your hand.
 
Oh, I never supposed that it might be a mistery. I thought the replies would be like "oh, I didn't knew, sorry", and/or "Timeslip is (or I am) definitely a man/woman".

Anyway, I'm going to refer to Timeslip as "he", since it's a little gender-neutral, besides some women even prefer to be called "he" (my sister does), and it would be very inappropriate to call a man "she".

By the way, can't you simply test Haenlomal's solution in-game? Just make the script change (it will take a minute, right?), go to New Reno with a new character, go boxing, and then view his resistances via character editor? Maybe hack him a little bit, to get to New Reno easier.
 
Darek said:
I'm not sure if I understand what you mean correctly, but calling wield_obj_critter with a piece of armor will make you wear it in the armor slot, and not only "holding it" in your hand.
Yes, that was what I meant, but that's still a bit surprising. I didn't see any special code to force armour to be worn rather than held.

In that case Haenlomal's fix should be perfect. :)
 
Darek said:
Josan12 said:
Hehe - the mystery continues. I think we should set Detective Darek on the case to find out for us. If anyone can, Darek can.
I just asked the magic eight-ball if Timeslip is a woman.
"Without a doubt" was the reply. So there you have it, case closed. :P

:lol: I do the same thing with all of my coding - "Is there a problem with my code?". My Magic 8-Ball always says "No". Methinks it's broken. :P

Darek said:
Timeslip said:
Now that I've actually bothered to read the function description in the mapper manual, it says that whatever you equip always ends up in the hand, without mentioning anything about armour. I didn't see anything in the function to do anything other than weild the item, so presumably calling wield_obj_critter with a piece of armour causes the critter to hold the armour rather than wear it. In that case, the ac bonus shouldn't be applied ever, because that function never actually causes you to wear the armour. (And the inven_worn bit isn't needed either, since it just stores away the old piece of armour for when adjust_ac is called.)

Unless the function description is wrong, all that's required to fix the bug is the line 'BlockCall(0x45697F);'.
I'm not sure if I understand what you mean correctly, but calling wield_obj_critter with a piece of armor will make you wear it in the armor slot, and not only "holding it" in your hand.

Yeah, the description that comes with the Mapper documentation is sometimes a bit off. Like it says at the top of Page 9:

Fallout_Editor.doc said:
The following was a list of potential script commands that may prove useful for programmers and scripters out there in the fan community who are trying to decipher what does what. I do not know what the gray-shaded definition blocks mean, but it could be significant as you're tearing through the guts of these commands. Maybe gray-shaded means these commands are diseased. Or don't work. Or got cut. Explore and find out, or wait for Red! and some crazy Team X Russians to figure it out.

Note that wield_obj_critter is one of those "gray-shaded" definitions, so I wouldn't be surprised if the function was meant to force the player to wear the armor. Especially since there seems to be no other script command that can force the player to do so.

-- The Haen.
 
Blackened said:
Anyway, I'm going to refer to Timeslip as "he", since it's a little gender-neutral, besides some women even prefer to be called "he" (my sister does), and it would be very inappropriate to call a man "she".

At least in American-English, "he" is not at all gender neutral, and for the most part, the only girls that would want to be called "he" would be transgender ones. I had no idea this was different elsewhere. Interesting.
 
Blackened said:
Besides some women even prefer to be called "he" (my sister does), and it would be very inappropriate to call a man "she".
What the hell is wrong with your sister? :shock:
 
Hey! Stop derailing my thread. :naughty:
If you guys wanna talk about Blackened's sister and such, may I suggest you take it to a more fitting place. :lol:

Nah, just kidding :P
 
Timeslip said:
Darek said:
I'm not sure if I understand what you mean correctly, but calling wield_obj_critter with a piece of armor will make you wear it in the armor slot, and not only "holding it" in your hand.
Yes, that was what I meant, but that's still a bit surprising. I didn't see any special code to force armour to be worn rather than held.

I think the armor is actually worn in the call to inven_wield, which occurs in offset 0x0045693C. inven_wield is a wrapper function for function invenWieldFunc (it merely sets a flag to 1 before passing it and all its arguments on through to invenWieldFunc), where the actual task of wearing the armor seems to happen.

This may be a potential problem, though. invenWieldFunc also calls adjust_ac, but fortunately it does so under proper conditions (i.e. get_item_type returns item_type_armor). Therefore, with my fix, adjust_ac is ultimately called twice, which may in turn lead to double the armor bonus.

Then again, Darek has stated that he tried to use wield_obj_critter with dude_obj and an armor item, and reported no unusual behaviour. With or without the fix, the armor item would guarantee that adjust_ac is called twice. Since there seems to be no problems, it looks like adjust_ac merely sets the bonus to ac, dr, and dt, overwriting whatever was the previous bonus. So hopefully things are still fine. :roll:

Hmmm...maybe I should take a deeper look into adjust_ac to see exactly what it does.

-- The Haen.
 
Haenlomal, any news on your progress?

I just installed Ollydbg to try and have a look myself. With the help of your explanations I could follow what's going on, but without that I would have been lost.
My respect for your work just went way up. :clap:
(That the programs help files don't work, doesn't exactly make it any easier.) :?
 
Darek said:
Haenlomal, any news on your progress?

I just installed Ollydbg to try and have a look myself. With the help of your explanations I could follow what's going on, but without that I would have been lost.
My respect for your work just went way up. :clap:
(That the programs help files don't work, doesn't exactly make it any easier.) :?

Well, the vast majority of that credit should go to Timeslip. I got most of what I know now about the engine code directly from hints and suggestions he made. :)

As for the issue itself: I'm currently on vacation right now (logging in on a public terminal to check on a few things), so I won't be looking into it until I get back around the middle of next week. I'll say a bit more when I have time to thoroughly tear through adjust_ac.

-- The Haen.
 
Quote "Gordulan"


noticed some weirdass shit after the boxing ring, I have a lvl 10 character, tagged, speech, sg and unarmed.

Perks, toughness awareness, and magnetic personality.

after becoming a prizefighter I noticed that I have 14% damage resistance, 14 damage threshold for normal damage and 5% damage resistance against all other damages when unarmoured, effectively overpowering my character against minigunners and conventional small arms, is it really supposed to happen in that way, cause according to the wiki the prizefighter "perk" gives you +5% normal resistance...?
 
Back
Top