Author Topic: Enemy attacks don't respect long-rangedness (FFVII). Can we change this?  (Read 8433 times)

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Edit: This issue has a patch. See http://forums.qhimm.com/index.php?topic=14998.0

Only recently, I discovered that all enemy attacks are treated as close range by the FFVII battle engine. Attacks that are not tagged close range, and attacks that are magical, will still perform half damage against player characters in the back row.

This is a bit of a problem for the balance of certain mods; allowing certain characters (e.g. those with long range weapons) to take half damage for the rest of the game is pretty broken.

The question is: what's the best way to handle this? I can think of two ways to try this:

  • Put everyone in the front row, disable the row change effect on the mag-rod enemy attack, and find a way of disabling the 'change' and 'order' commands
  • Force a check in the damage calculation for the short-range flag; if the attack doesn't have the flag, skip the row check function
What is best and most feasible? My gut instinct tells me that opting for 2 is the best choice, because it fixes the intended behaviour and creates a patch for other modders to use in other versions of the game.

What I don't know is exactly how to do this. I believe that the battle executable is in batini.x, and that the file is basically gzipped. I could probably get the asm out of it. Finding the relevant functions might be hard, though. I know the location of the enemy stats in the PSX's memory, so I guess I could look for a read and assume that read is the first part of the initial base damage equation. I guess I could try and follow what the game does from there. If I can locate the data for the enemy attack power, and all the attack stats are together, I could probably work out how to lookup the flag. If I need to make the binary bigger, it looks to me like the LBA+Size of batini.x are in the YAMADA.BIN (see the string 63 45 00 00 40 16).

How feasible is this? Is there anything I've missed? Has anyone else already looked at this problem and have any additional insights?


Another thing - how do I use the call stack info of the PSX emulator's debugger? What are the values in the stack viewer? Are they the locations the CPU are expected to return to once the function has finished? Is the most immediate function call at the top (as per a stack?)
« Last Edit: 2014-02-12 22:09:26 by Bosola »

nfitc1

  • *
  • Posts: 2981
  • I just don't know what went wrong.
    • View Profile
    • WM/PrC Blog
The short-range wasn't designed for enemies. Most of them have arbitrary rows assigned to them ONLY for use of the binary cover flags. Other than that they are just positioned at arbitrary Z coords.

Also, the current code checks for damage of player actions I don't think enemy attacks are affected (Being command index 20h). Altering this might be complicated. Also, when a player is a target of a back-row enemy, do they take quarter damage? It gets tricky to determine if damage has already been deducted.

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Quote
when a player is a target of a back-row enemy, do they take quarter damage? It gets tricky to determine if damage has already been deducted.

I don't really think this should matter; all I want to do is make long range enemy attacks ignore player character rows. I don't need enemies in different row positions to deal differing amounts of short-range damage; it's 'long ranged-ness' I'm adding, not extra behaviour around short-range attacks. Presumably the existing function already does a check on player row - all I really want to do is skip this function call if the attack doesn't have the 'short range' flag enabled. Otherwise, the existing behaviour can continue however it's coded (though there's probably quirks with that stuff, too).

Do you have any other ideas for solving this problem?
« Last Edit: 2014-02-03 07:09:06 by Bosola »

nfitc1

  • *
  • Posts: 2981
  • I just don't know what went wrong.
    • View Profile
    • WM/PrC Blog
Ah, yes. That's an easy one.

There's a pair of ASM commands that forcibly restricts enemy actions (Command Index 20h) from having this applied during damage calculation. I know you do a lot of PSX editing so I don't know how this will translate, but the "offending" code is at 0x5DE704. It selectively blacklists the 20h command from receiving long-range consideration. Changing the command index checked to something out of range would be ideal:

0x5DE704: 83 78 28 20  ->  83 78 28 50

Oddly, it checks if the target is in the back row before even checking if the action is short/long range. Doesn't seem efficient.

allowing certain characters (e.g. those with long range weapons) to take half damage for the rest of the game is pretty broken.

To be "fair", those characters are optional.

Kaldarasha

  • *
  • Posts: 2450
  • Prince of Model Editing
    • View Profile
Is it possible to add an accuracy penalty for physical attacks for the back row?

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Ah, yes. That's an easy one.

There's a pair of ASM commands that forcibly restricts enemy actions (Command Index 20h) from having this applied during damage calculation. I know you do a lot of PSX editing so I don't know how this will translate, but the "offending" code is at 0x5DE704. It selectively blacklists the 20h command from receiving long-range consideration. Changing the command index checked to something out of range would be ideal:

0x5DE704: 83 78 28 20  ->  83 78 28 50

Hmm, I'm going to be working on the PSX version. Are the algorithms the same? When trying to track down this action, is there something I should look for? When about in the damage equation is this check taken?

Some of the battle engine is disassembled at http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/ffvii_battle.asm - around 9163 and 9650 there's some stubs for 'battle_main_damage_calculation' and 'battle_damage_formula_run' - I assume these are the candidates to reverse? I'll try and get the ASM out of the overlay and see how they work.

Quote
To be "fair", those characters are optional.

You forget about Barret!

nfitc1

  • *
  • Posts: 2981
  • I just don't know what went wrong.
    • View Profile
    • WM/PrC Blog
Hmm, I'm going to be working on the PSX version. Are the algorithms the same? When trying to track down this action, is there something I should look for? When about in the damage equation is this check taken?

Some of the battle engine is disassembled at http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/ffvii_battle.asm - around 9163 and 9650 there's some stubs for 'battle_main_damage_calculation' and 'battle_damage_formula_run' - I assume these are the candidates to reverse? I'll try and get the ASM out of the overlay and see how they work.

It's immediately after the actual damage calculation for X1 damage type. I have no idea where in the PSX that would reside.

You forget about Barret!
No I didn't, some of Barret's weapons are short range and his ultimate won't have consistent damage in all materia builds.

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Perhaps I could look there first. If I can actually open the file property - I've managed to decompress it, and I've opened it in LemASM, but I can't match the functions in the reversing doc with the assembler I'm looking at. Am I supposed to parse the binary in little- or big-endian format?

Edit: Aha! Apparently, I needed to do a wordswap as well as a byteswap. I didn't know the words were read in reverse. Now the disassembled file looks a lot like Akari's reverse. For the sake of any future reader, I found that the point 0x32530 in the decompressed file matches up with line 26789 in the reverse, or 0x800D2530 in the PSX memory map. It's a distinctive jump opcode in a sea of NOPs.

Edit 2: I think I've found the assembler in question. It's at 0x0000DB00 in the decompressed BATTLE.X:

Code: [Select]
30 42 00 20 ANDI V0, V0, $0020
10 40 00 03 BEQ V0, R0, $0000DB14

I found this by looking at the excellent Q-Gears resources, under the 'reverse' folder. The function http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/damage/damage_lower_calculation_function.cpp showed that the branch for applying back row modifiers was straight after the main X1 damage equation, just as in the PC version. Then, using http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/ffvii_battle.asm as my guide, I found that the function for lower_01 damage resides at 0x8000D944.

From there... well, I cheated. I don't know MIPS assembler, but I do know what registers to look out for (the Q-Gears source helpfully uses them as local variables) and I know what the branch statements look like. I also know about some hardcoded numbers that I could look out for whilst eyeballing the source. From there, I could find a patch I could examine a lot closer.

Of course, none of this may have worked - I've yet to try actually modifying the source, and I could be totally wrong. I can't wait to give it a go though!

Edit3: Huzzah! It works! Setting the 20 to 50 re-enables the check. Only I now just realized... I don't want to re-enable the check; enemy attacks are already tested for range. I want to disable row effects for long range enemy attacks. Changing DB00 causes long range attacks to take quarter damage, because now row effects are being considered twice.

I feel so thick. I'm going to pick this up again tomorrow with a clearer head; I'm far too tired to continue tonight.

The odd thing is, when I look at http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/damage/damage_lower_calculation_function.cpp#l46, I can see that the enemy back row status is considered, and that the user back row status is considered, but no checks around the long-rangedness of the attack. I need to find out where this is being handled.
« Last Edit: 2014-02-04 22:38:57 by Bosola »

nfitc1

  • *
  • Posts: 2981
  • I just don't know what went wrong.
    • View Profile
    • WM/PrC Blog
... I don't want to re-enable the check; enemy attacks are already tested for range. I want to disable row effects for long range enemy attacks. Changing DB00 causes long range attacks to take quarter damage, because now row effects are being considered twice.

I feel so thick. I'm going to pick this up again tomorrow with a clearer head; I'm far too tired to continue tonight.

The odd thing is, when I look at http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/damage/damage_lower_calculation_function.cpp#l46, I can see that the enemy back row status is considered, and that the user back row status is considered, but no checks around the long-rangedness of the attack. I need to find out where this is being handled.

???

That snippet that I said to change was the long-range consideration. It goes (on the PC version at least):

Row of target
Short-range attack
Row of attacker

The snippet that determines the short-range-ness of the attack was what was blacklisting the enemy command.


EDIT:
OK, I was a little confused at the code. I haven't looked at ASM in a while so it threw me off. The problem is more complex than I thought. The wiki has it mostly correct (yes, I wrote this part):

Quote
Long_Range = 0
If Target is in Back Row
   Set Long_Range = 1
End If

If Attack is Short Range OR Command is Enemy Attack
   If Actor is in Back Row
      Long_Range = 1
   End If
Else
   Long_Range = 0
End If

If Long_Range != 0
   Damage = Damage / 2
End If

That's slightly confusing, but let me clarify.

It checks the target's row first, but only players are ever flagged as being in the "back row".
Then if the attack is short range it will check if the actor is in the back row. THAT will fail for an enemy because enemies are never flagged as being in a "back row".
If the attack is short-range then it doesn't check the command index. If the attack is long range and the command index is not 20 then it won't check the actor's row. That's what that change I told you to do altered. However, enemies are never in the back row so that doesn't matter as the next check would fail.

If I counted correctly, we end up with 18 scenarios:

Code: [Select]
1.  Enemy        attacks Front-player with short range
2.  Enemy        attacks Front-player with long range
3.  Enemy        attacks Back-player  with short range
4.  Enemy        attacks Back-player  with long range
5.  Enemy        attacks enemy        with short range
6.  Enemy        attacks enemy        with long range
7.  Front-Player attacks Front-player with short range
8.  Front-Player attacks Front-player with long range
9.  Front-Player attacks Back-player  with short range
10. Front-Player attacks Back-player  with long range
11. Front-Player attacks enemy        with short range
12. Front-Player attacks enemy        with long range
13. Back-Player  attacks Front-player with short range
14. Back-Player  attacks Front-player with long range
15. Back-Player  attacks Back-player  with short range
16. Back-Player  attacks Back-player  with long range
17. Back-Player  attacks enemy        with short range
18. Back-Player  attacks enemy        with long range

Working out the logic to this the result is:

Code: [Select]
1.  Target not in back row; attack is short; N/A;                         actor not in back; damage NOT reduced
2.  Target not in back row; attack is long;  command is enemy attack;     actor not in back; damage NOT reduced
3.  Target is in back row;  attack is short; N/A;                         actor not in back; damage IS reduced
4.  Target is in back row;  attack is long;  command is enemy attack;     actor not in back; damage IS reduced*
5.  Target not in back row; attack is short; N/A;                         actor not in back; damage NOT reduced
6.  Target not in back row; attack is long;  command is enemy attack;     actor not in back; damage NOT reduced
7.  Target not in back row; attack is short; N/A;                         actor not in back; damage NOT reduced
8.  Target not in back row; attack is long;  command is not enemy attack; N/A;               damage NOT reduced
9.  Target is in back row;  attack is short; N/A;                         actor in back;     damage IS reduced
10. Target is in back row;  attack is long;  command is not enemy attack; N/A;               damage NOT reduced
11. Target not in back row; attack is short; N/A;                         actor not in back; damage NOT reduced
12. Target not in back row; attack is long;  command is not enemy attack; N/A;               damage NOT reduced
13. Target not in back row; attack is short; N/A;                         actor is in back;  damage IS reduced
14. Target not in back row; attack is long;  command is not enemy attack; N/A;               damage IS reduced
15. Target is in back row;  attack is short; N/A;                         actor is in back;  damage IS reduced
16. Target is in back row;  attack is long;  command is not enemy attack; N/A;               damage IS reduced
17. Target not in back row; attack is short; N/A;                         actor is in back;  damage IS reduced
18. Target not in back row; attack is long;  command is not enemy attack; N/A;               damage IS reduced

Scenario 4 looks suspect because the action is a long range one. That SHOULD cancel out the rest of the checks, but it doesn't. So what it SHOULD do to fit the pattern correctly is check the range of the action first. If it's long range then you shouldn't care about the rows of the actor or target, yet this does. Clearly they wanted enemies to NEVER do full "X1 damage" to a player in the back row. The only action I can imagine that truly affecting is Goblin Punch. It's the only X1 damage attack that both players and enemies (albeit only one can do it) have access to. It's a long-range attack so players will do full damage in the back row, but take half damage when a Goblin does it to them. This has the side-effect of all enemy actions being short-ranged.

Anyway, to fix this would require an entire re-write of this section. I could do that on PC because I have it. I can't tell you what to do for the PSX version, however.


Is it possible to add an accuracy penalty for physical attacks for the back row?

The easiest way I can think to do it would be to copy the Fury accuracy adjustment method to a larger block of unused memory and factor in the range there. Even that as it is, not an easy task.
« Last Edit: 2014-02-05 19:20:23 by NFITC1 »

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Hmm. This doesn't look like a _horribly_ hard rewrite given that I now know what the values represent. So long as I use the same registers and leave them in the same state I think I can do this, albeit with a lot of references to the instruction set docs. Famous last words, though.

The bigger question, though, is why my edit made the attack deal 1/4 damage rather than 1/2.
« Last Edit: 2014-02-08 12:10:45 by Bosola »

Bosola

  • Fire hazard!
  • *
  • Posts: 1748
    • View Profile
    • My YouTube Channel
Bump for new information.

So I've been investigating the procedure at length with LemASM and Halkun's PSX guide and I think I have a fix.

Firstly, it looks like my earlier information about the position of the opcode was incorrect. The instruction at DB00 is actually part of the 'isDefending' check, which is probably why messing around with it quartered my damage. No, the procedure for checking back-row-ness starts at 800ADA28 (DA28) and terminates at DAC4. As NFITC1 says, it has a flaw, namely that if an attack is an enemy action, the branch for a long range attack is effectively exited. Here's my own low-level reverse (this took a while to get the logic right; I didn't realize jump and branch opcodes had a one-instruction delay):

Code: [Select]
// Start at DA28
// R4 contains result of first damage equation

R3 = load word from [r5 + 208] // gets value of offset 208 from temporary scenario array stored in R5
R2 = R3 * 2
R2 = R2 + R3
R2 = R2 * 4
R2 = R2 + R3
R2 = R2 * 8
R1 = 8010 0000
R1 = R1 + R2
R2 = load word from R1 + 83E4
// this seems to get the target unit flags specified at http://sourceforge.net/p/q-gears/code/ci/default/tree/reversing/ffvii/ffvii_battle/ffvii_address.txt#l655
// I *think* these are flags for the target rather than the actor
R3 = load word from [R5 + 0050] // get attack target flags
R2 = R2 && 0040 // R2 = target.isBackRow
R3 = R3 && 0020 // R3 = attack.isShortRange
R6 = (0 < R2) // reducedDamageFlag = target.isInBackRow
IF R3 != 0, BRANCH TO DA74

// DA64
R3 = load word from [r5 + 28] // get attack type data
R2 = 0 || 20 // 'enemy attack' command
IF R3 != R2, BRANCH TO DAB4

// DA74

R2 = load word from [r5 + 0]
R3 = R2 * 2
R3 = R2 + R3
R3 = R3 * 4
R3 = R3 + R2
R3 = R3 * 8
R1 = 8010 0000
R1 = R1 + R3
R2 = load word from [R1 + 83E4]
// seems to be unit flags for actor
R2 = R2 && 0040 // R2 = self.isBackRow
IF R2 = 0, BRANCH TO DAB8

R6 = 1 // reducedDamageFlag = true
JUMP TO DAB8

// DAB4
R6 = 0

// DAB8
R2 = R4 >> 1F
// right shift R4 - current damage - by 31 places, leaving upper bit
IF R6 = 0, BRANCH TO DAC8
R2 = R2 + R4
// add upper bit to get negative damage to bitshift divide properly?
R4 = R2 / 2

// DAC8
// Seems to be start of ‘target defend’ check

This is a bit hard to follow. Here's a digested version:

Code: [Select]
applyDamageReduction = target.isBackRow;
if (attack.shortRange) {
    checkActorRow();
}
else {
    // if attack is long range
    if (attack.enemyCommand == false) {
        applyDamageReduction = false;
    }
    else {
        checkActorRow();
    }
}

function checkActorRow() {
    if (actor.isBackRow) {
        applyDamageReduction = true;
    }
}

if (applyDamageReduction) {
    divideCurrentDamageByTwo();
}


Basically, if the attack is long range and is an enemy attack, the flag is not cleared, and the actor's row is still checked regardless of the fact the attack shouldn't care about rows.


This won't do - as it means enemy attacks are having their long-range status ignored. What we actually want is this:



My fix

This is the fix I'm trying, at a high level. It's pretty basic:
  • Get attack target data; if long range, skip to defend check script (DAC8)
  • Store self.backRow in R1
  • Store target.backRow in R2
  • If R1 || R2 = false, skip to defend check script
  • Apply damage reduction to R4
There's a bit of redundancy here, but this is my first ASM patch, so I really don't care about a few wasted cycles.

Here's how I think I can write the assembler:

Code: [Select]
// Check for long range attack

LW R1, $0028 (R5)      ;load R5+28 (target flags?) into R1
ORI R2, R0, $0020      ;don't know why the original code used this method, but I'll preserve it
BNE R1, R2, $0000DAC8 ;goto defend script
NOP                            ;do I need to include these after jumps?

// Load self.backRow into R1
LW R1 $0000 (R5)     ;load R5+0 (self unit flags?) into R1
NOP                           ;apparently loads are delayed by one cycle
SLL R2, R1, $1
ADDU R2, R2, R1
SLL R2, R2, $2
ADDU R2, R2, R1
SLL R2, R2, $3
LW R1 $83E4 (R2)     ;get offset and load
NOP
ANDI R1, R1, $0040  ;this should put the AND of R1 && 0x0040 into R1

// Load target.backRow into R2
LW R2 $0208 (R5)   ; load R5+208 (target unit flags?) into R2
NOP
SLL R3, R2, $1
ADDU R3, R3, R2
SLL R3, R3, $2
ADDU R3, R3, R2
SLL R3, R3, $3
LW R2 $83E4 (R3)      ;get offset and load
NOP
ANDI R2, R2, $0040      ;this should put the AND of R2 && 0x0040 into R2

// Do comparison
// If either self or target are back row, their register should contain 0x0040
OR R3, R2, R1           ;will contain 0x0040 if either party in back row, should contain 0x0000 if not
BLEZ R3, $0000DAC8     ;skip to defending script if zero

// Continue with existing 'reduce damage' implementation, ignoring test for R6 != 0

I'll try the above this afternoon. I'll double check, but I think this should be right. My only hope is that I don't make the procedure longer; I'd rather not mess around moving the file. I think it should be about the same, though. I assume ARMIPS will just overwrite from the address I specify, so I won't need to update any other jumps in the procedure? In terms of lines, it'll certainly make things smaller. Might be an idea for me to fill in any unused space with NOPs though.

I should probably also double check that enemy attacks get their target flags copied the same way too...

One final question: if I write branches, jumps or any other instruction-delayed opcodes in my assembly and pass it to ARMIPS, will the assembler insert the NOPs automatically or do I need to do this myself?

Edit

Excellent! I was able to compile my assembler, and it looks as though it was written to the file just fine. Had to make a couple of changes here and there (I didn't realize that branches had a reach limit, for example), but it seems to compile without trouble. Here's the actual script that I used with ARMIPS:

Code: [Select]
.open "BATTLE",0x800A0000
.psx

.org 0x800ADA28

.area $9C

//check_long_range_attack
   lw   v1, $0050(a1)
   nop
   andi   v1, v1, $0020
   bgtz   v1, check_self_backrow ; if short range attack, check self/target backrow
   nop
   j   0x800ADAC8 ; else skip to next sub

check_self_backrow:
   lw   v0, $0000(a1)
   nop
   sll   v1, v0, 1
   addu   v1, v1, v0
   sll   v1, v1, 2
   addu   v1, v1, v0
   sll   v1, v1, 3
   lui   at, $8010
   addu   at, at, v1
   lw   v0, $83E4(at)
   lw   v1, $0208(a1) ; start loading word for check_target_backrow (save space)
   andi   v0, v0, $0040 ;v0 now contains self.isBackRow

//check_target_backrow   
   sll   at, v1, 1
   addu   at, at, v1
   sll   at, at, 2
   addu   at, at, v1
   sll   at, at, 3
   lui   v1, $8010
   addu   v1, v1, at ;v1 contains address for lookup
   lw   at, $83E4(v1)
   nop
   andi   v1, at, $0040 ;v1 now contains target.isBackRow

//do_comparison
   or   at, v0, v1
   bgtz   at, reduceDmg ; if target.isBackRow || self.isBackRow, go to reduceDmg branch
   nop
   j   0x000ADAC8 ;skip to next sub
   nop

reduceDmg:
   j   0x000ADABC ; sends to damage reduction subfunction
   nop

.endarea

.close

The endarea tests for the size of the overwrite (quite a handy feature). It looks as though my writeup is actually the same size.

Unfortunately, I haven't been able to test it in battle yet, as I'm having trouble reinserting the file. The game doesn't seem to like my gzipped insert; starting a battle freezes the game and looking in memory for the updated assembler suggests the decompression has gone *badly* wrong. Anyone have any ideas?

Edit again: I fixed the issue with the file inserts (I was _overwriting_ the first 8 bytes of the gzipped file with the x file header, rather than _inserting_ the original 8 bytes back in again). It's now working, except the logic seems to be off. I think I got the address wrong in one case - looks like R5+0050 is what gets the targeting flags, whereas R5+0028 gets the command type. Changing my script makes things work better, but now it looks like everything is being treated as long range...I will trace and report on this soon.

Edit2: I have fixed the issue using the updated code above. I have raised a new thread about this fix (http://forums.qhimm.com/index.php?topic=14998.0). I'll also share an end-to-end guide to how I performed this hack, so others wondering about PSX assembly edits can get some much-needed information.
« Last Edit: 2014-02-12 22:08:31 by Bosola »