Final Fantasy II Restored

[redmagejoe]

Status Update: I should be free by Monday and will begin looking into the Ripper/Drain/Heal Staff fix and see if I can't help locate where battle poses are being updated in code, and see if there's any way to fit those into the existing Drain fix. Hoping to hear from abw soon and see if he can answer any of my questions about the RNG thing, since as I said, I'm really not that learned on CPU timing and how it plays into the code.

[abw]

But I also wish there was some way I could contribute to the RNG timing issue leading to the graphics bug, I'm just completely unlearned in the concept. I'm assuming the issue is that the code has to work within a specific number of CPU cycles to not elbow in on VRAM time, and thus one has to know how many CPU cycles the currently updated RNG is taking.

I wasn't able to reproduce any graphical glitches (during level ups or otherwise) using the RNG patch, but I was able to consistently trigger glitches on level ups using the Level Up Routines Rework patch; I added another sprite 0 check there, and I'm not seeing glitches anymore, but I haven't really delved into that part of the code, so it might still require further tweaking. Just in case it was an issue somewhere, I also updated the RNG patch to restore the early kickout when the lower and upper bounds are the same, which drastically speeds up those cases and better reflects the original code's consumption of numbers from the base RNG lookup table.

Is the reason the old RNG was so poorly optimized because it was taking inherently less time?

From an optimization perspective, the old RNG wasn't that bad; the worst thing that comes to mind is that it performs 16-bit multiplication when all it really needs is 8-bit multiplication, so that part could easily be done twice as quickly.

This is one of the best ideas I've seen for the game and something the remakes never addressed. The tedium of leveling up spells and weapons is enormous and Battle Rank seems to mean next to nothing. I think Battle Rank bonus/malus should be applied across the board for any action taken so that you can't successfully grind off of Battle Rank 1 to get 9999 HP, but that you aren't still leveling up Flare to Level 4 in Pandemonium.

It would take a bit of a rewrite and a lot of checks to the Battle Rank but I would love to see a hack which addressed these flaws. It would change the flow of the game completely, yes, but it might also cut down on the tedium of grinding endlessly.

Currently, exp gain for any skill with at least 1 action (this includes Evasion and Magic Resist skill levels in addition to weapon/spell levels) is calculated as

Code Select Expand

(base growth amount for type of skill) + (# of actions) + (battle rank) - (base growth barrier for type of skill) - (skill level) where the result is subject to various quirks caused by the game's missing under/overflow checks. For spells specifically, allowing a chance for growth with 0 actions reduces that to

Code Select Expand

2 + (battle rank) - (skill level) so spells with no actions would passively level up to battle rank + 2 over time, which is at least in the neighbourhood of reasonable and can be adjusted fairly easily.

For stat growth,

• HP/MP/Stamina/Magic Power growth chance is based only on whether HP/MP loss ratio beats a random number, with HP/MP growth amount = Stamina/Magic Power
• Strength/Intellect/Spirit growth chance is based only on whether your attack counter beats a random number
• If you gained the opposing stat, Intellect/Stamina/Strength loss chance is based only on whether a random number beats 0

Working battle rank into those calculations wouldn't be too hard; higher battle ranks could give you a lower minimum HP/MP damage ratio and a more favourable required number of attacks. On the other hand, stat growth doesn't feel like as much of a problem to me as skill growth does. Although it's not directly used in the calculation, I find that HP/Stamina in particular tends to scale pretty well with battle rank already since higher battle ranks tend to dish out more damage.

(Also I wouldn't mind changing some of the spells into combat moves as a reward for leveling up weapon types. (I mean Sap, Swap? What were they even thinking with Swap other than a way to abuse their poor mechanical setup?) get rid of Toad, Mini, and Frog, the game does not need three more instant death spells... Wall is weird in the first place. Apparently it blocks a singular magical attack... sometimes. I do know that there are some enemy only spells as well which could be used for that.

I haven't tested it myself yet, but based on reading the code, it looks like casting Wall on somebody with a given number of successes should negate all black magic with up to that number of successes; Wall should persist until its target gets hit by some black magic with a higher number of successes, so e.g. Wall with 16 successes ought to make its target immune to all black magic for an entire battle, making it pretty great (unless you want to use beneficial black magic like Berserk/Haste/Aura ).

For the v2.0 of Chaos Rush translation, I'm wondering if it would be easier to simply extend the display box for spells by one more character rather than trying to redo the graphics used for the 5-character long spell names. This box extension would be specific to the translation patch, not the Restored patch.

This is probably the "right" way to do it, but will require figuring out how the display boxes are drawn, what determines their width, and presumably whether whatever RAM space they're using to buffer the text is large enough to hold more tiles. Definitely worth investigating, though.

If we had more room in ROM, I might consider having messages for those leveling up, but since it didn't do that in the remakes (I think, would have to test this on PSP version), and it may become involved, I'll put that at the end of the Possible Improvements list. I think it's a worthwhile improvement, I just don't know how feasible it is to implement with limited space, but the groundwork to implement it is definitely there.

Battle messages are stored and used in bank 5, which does have enough free space to be useful (and we've made more), so this might not be too difficult.


In other news, I've also gone through and commented large swaths of the bank C code including the player-castable spells; this led to the discovery of some more bugs:

• The bonus amount from Protect and Berserk can overflow, but the code doesn't check the high byte of the multiplication result, so if you score +256, you actually get +0.
• Like Aura and Barrier, Dispel's battle messages are in reverse order compared to its effects.
• Aura, Barrier, and Dispel all display battle messages based only on their number of successes, not on whether their successes actually made any difference. E.g. if you already resist Fire and score at least 2 successes, Barrier still says your Fire defense increased even though it's exactly the same as it was before.
• After determining how much HP to restore, Life only writes the low byte of the result to your HP, which means it can leave you with 0 HP. In this case, the target's death message is displayed before Life's success message and the target remains dead.

I'm not sure how relevant this is, but Hironobu Sakaguchi explained why the Ultima bug is a thing: when he first discovered it, he wanted the bug fixed, but some jerk programmer said no, because the bug reflected how legendary weapons/items in real life rarely live up to their own hype. Then, he ciphered the bug, so Sakaguchi couldn't fix it himself. So, here's to hoping you don't have too much trouble fixing Ultima.

Still working on my work project, but thought I'd pop in to say that, given that Ultima behaves the way Sakaguchi wanted it to in the remake, and knowing that was his original intent, it is definitely a priority to attempt to make it work in this version. Perhaps that will be our Final Challenge.

I went and looked into this, and it was the source code that was ciphered apparently. Fortunately we're reverse-engineering it, so I'm not sure it would be an issue, though if by "source code" it means the assembly which we're turning the binary back into... Then yeah, we may have trouble. Hopefully we can make sense of most of the rest of the disassembly such that the only things left by the time we get to it must, by process of elimination, be related to Ultima.

Ultima does appear to be intentionally bugged, but its final assembly is no more complicated than any other spell's, so if there was any ciphering going on, it must have happened at a higher level in their build process. Ultima's problem is that, aside from the modifications to Spell Success % and Spell Power based on the caster's INT/SPR that apply to every spell, it does not scale at all. In particular, it does not scale with the level of your other abilities as in later remakes nor does it even scale with its own level like other spells do; basically, instead of using spell level as the number of success attempts, you get 1 guaranteed success + 1 or 2 resistable successes.

These aren't specifically related to spells, but there are some other things I've noticed that could be considered bugs:

• HP/MP loss ratios are calculated based on net HP/MP loss rather than total HP/MP loss, which has the effect of punishing the player for healing during battle. It looks like there might be some unused bytes in each character's skill data, so possibly we could use 4 of them per character as a running total of inflicted HP/MP damage, optionally restricted to damage inflicted by a hostile source (for HP anyway; only giving credit for enemy-inflicted MP damage would be game-breaking), and optionally reset when gaining a stat instead of at the start of every battle. Restrictng HP/MP loss ratios to damage inflicted by an enemy would eliminate attacking your own party and abusing Swap as grinding exploits. Carrying losses across battles would allow them to accumulate over time and eventually guarantee stat increases, as opposed to the current system where you can suffer 10% losses per battle for 10 battles with 0% chance for growth.
• There's a buffer overflow when more than 20 battle messages are queued (which is possible with e.g. 2 weapon skill increases + 16 spell increases + 8 stat increases + 3 stat decreases = 29 queued messages); the game reserves $7FBA-$7FCD for battle messages but uses $7FCE+ for other currently unknown things and none of the code for adding battle messages to $7FBA,X checks for X > #$13. Based on analysis so far, I think the only time you can trigger overflow is at the end of a battle and I think $7FCE+ isn't used afterwards, so maybe it's harmless?
• Lending further support to the idea that dual-wielding should increase your combat offense, Joseph's initialization data starts him with 2 fist attacks at level 2 = 4 hit attempts, but that gets reduced to 2 hit attempts at the first time his calculated stats get updated.

[redmagejoe]

God that's a lot to take in. I could swear that I just ran a version of the base game with the RNG fix and nothing else and saw those bugs but I could check again... Anyway, if you've got an updated Level Up Routines Rework patch, I'd like to have a look and play around with it. I updated your RNG fix with the stat loss value changes I put in the previous one, to avoid the increased chance to lose stats. I just finished my work project and I'm kind of... turning my brain off for a bit so I don't get burnt out. Tomorrow I'll probably work on this stuff again.

As stated, I'm settling with just removing the space in the Chaos Rush version for now, as I really don't want to redo the spell name character graphics, and I don't want to mess with window borders either. I'm glad to hear that we have room for our messages, so I'll keep that in mind when we get near the end of our bug fix part of the project.

More bugs, more bugs... Have to add overflow protection to the calculated bonus of Protect and Berserk... I already fixed the message order of messages for Aura, Barrier, AND Dispel, both for this patch (to JP version), and for the Chaos Rush patch... Already know about their unconditional messages, but didn't take into account how it interacts with your existing resistances or enemy's non-existing resistances... Not sure that part is too important, since whether they were already vulnerable to a resist or not, if the spell says they are "NOW vulnerable" to X status, or you're protected... in the end, it's all the same, yeah? I don't know, again, head hurts, not thinking clearly so just rambling...

Life bug definitely needs to be addressed... I will give serious consideration to fully implementing dual wielding, and as for healing offsetting HP/MP growth, I'll add that to the list before I go take a nap. The fact that we have some bytes to work with makes that more appealing a prospect.

EDIT: Yeah my mistake, it seems that the RNG fix alone doesn't cause the problem. Maybe it was a combination of the two working together? I don't know. Anyway, do I need to insert a wait for sprite 0 hit (JSR $FD5B) or sprite 0 clear (JSR $FD46)?



Thanks, as always, to your awesome commenting of the disASM, abw, I think the Life bug fix is easily made. I'm assuming that, space allowing, we need only insert after 0x0338DD:

Code Select Expand


C8    INY    ; increments Y to #$0B, Battle stat offset for Current HP high byte
A5 05    LDA $05   
91 A1    STA ($A1),Y ; pointer to target's battle stats


Question is... where to get 5 bytes? A lot of spell bugs need to be fixed in this area, Sap (right after Life) included, so maybe we can make some space by virtue of fixing some of these? Though some fixes may need MORE rather than less space, so... Let's plan out fixes for them, then try to trim them down as much as possible, then look at how we can work with the available space versus the space we need.

Code Select Expand


0x0338FD|$0C:$B8ED:A0 0C    LDY #$0C    ; Battle stat offset for Current MP low byte
0x0338FF|$0C:$B8EF:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x033901|$0C:$B8F1:85 00    STA $00
0x033903|$0C:$B8F3:A9 00    LDA #$00    ; BUG: should operate on high byte of target's current MP too
0x033905|$0C:$B8F5:85 01    STA $01

===>

Code Select Expand


0x0338FD|$0C:$B8ED:A0 0C    LDY #$0C    ; Battle stat offset for Current MP low byte
0x0338FF|$0C:$B8EF:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x033901|$0C:$B8F1:85 00    STA $00
0x033903|$0C:$B8F3:C8 INY ; increment Y to #$0D, Battle stat offset for Current MP high byte
0x033904|$0C:$B8F4:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x033906|$0C:$B8F6:85 01    STA $01


Need +1 byte for the Sap fix, +5 bytes for the Life fix... what else can we quickly fix? Osmose fix may require enough space that we may need to hijack the JSR to a new one, and pass the old JSR through that one before returning back to our original control flow. Drain and Osmose use the same control block, and branches if we're dealing with Drain. Question is, do we have similar bugs with Drain (awarding more HP than taken from the enemy) or is that coded properly? I recall you saying that there was an issue with Drain not setting battle pose properly, so since we're already looking at it... Also likely need a JSR to a new sub from both Protect and Berserk to handle the case where Attack/Defense would be above 255 (or is it the bonus itself that's the issue?) since they both seem to use the same math.

Also, I suppose I should ask if the only issue with the weapon special effects fix is the Drain issue (which is inherent to this spell handler code rather than the weapon code itself), or did you manage to get Heal Staff working properly on Undead (doesn't heal them like it does non-Undead, deals the base attack damage * successful hits rather than the 0 it does on non-Undead, ALSO deals bonus dmg * successful hits to Undead, similar to Ripper)? If the only issue is with Drain, I'd like to add that patch to stable, assuming you feel it's working as intended.

Code Select Expand


; handler for ????, Spell ID #$D1: Berserk


Yoichi's Bow can be used in combat to cast Berserk on the party, if that's what the ???? is referring to.

[abw]

Anyway, if you've got an updated Level Up Routines Rework patch, I'd like to have a look and play around with it.

It's available at the same link as before, but since those are spread all over the place, here's a single link to everything I've uploaded for FF2!

As stated, I'm settling with just removing the space in the Chaos Rush version for now, as I really don't want to redo the spell name character graphics, and I don't want to mess with window borders either.

Yup, that's fine. I'm just saying that based on other games I've looked at, we might get lucky and be able to expand the width of the spell name window by changing a single byte somewhere.

More bugs, more bugs... Have to add overflow protection to the calculated bonus of Protect and Berserk...

As you've seen, the good news is that most of the bugs are simple to fix; more good news is that there is some easily shrinkable code in bank C. Ultima in particular is basically just inlining a bunch of other routines that already exist elsewhere, so rewriting that to act more like the remakes frees up a lot of space.

I already fixed the message order of messages for Aura, Barrier, AND Dispel, both for this patch (to JP version), and for the Chaos Rush patch... Already know about their unconditional messages, but didn't take into account how it interacts with your existing resistances or enemy's non-existing resistances... Not sure that part is too important, since whether they were already vulnerable to a resist or not, if the spell says they are "NOW vulnerable" to X status, or you're protected... in the end, it's all the same, yeah? I don't know, again, head hurts, not thinking clearly so just rambling...

Yeah, it depends a lot on the exact text of the messages. It looks like Chaos Rush's translations should be fine, but some of Demiforce's suggest that multiple applications of the spell might stack their effects, which is not the case.

Question is... where to get 5 bytes?

The closest source of that many free bytes looks like Swap. When it's capping the actor's and target's current HP/MP at their max HP/MP, it first sets $5C/$5D for HP then sets $5E/$5F to the actor, caps, then sets $5E/$5F to the target, caps, and then sets $5C/$5D for MP. At that point, $5E/$5F still refer to the target so you could save 8 bytes by immediately capping, but instead the game switches to actor, caps, switches to target, caps. I made a couple of notes about some other spots where space could be saved (search for e.g. $B571), but in general, a lot of the spell code is pretty similar and could be abstracted to save space. Take Protect/Berserk for example: aside from the constants for the offset of the stat to buff and the battle message to display, they're basically the exact same routine duplicated.

Also, I suppose I should ask if the only issue with the weapon special effects fix is the Drain issue (which is inherent to this spell handler code rather than the weapon code itself), or did you manage to get Heal Staff working properly on Undead (doesn't heal them like it does non-Undead, deals the base attack damage * successful hits rather than the 0 it does on non-Undead, ALSO deals bonus dmg * successful hits to Undead, similar to Ripper)? If the only issue is with Drain, I'd like to add that patch to stable, assuming you feel it's working as intended.

I still haven't got back to looking at the special effects fix yet. The healing from Drain/Osmose is equal to the damage inflicted, which can indeed exceed the target's max HP/MP, so we'll want to cap that. Heal Staff should be working correctly, but I'll need to retest all the special effects once I'm done updating.

Code Select Expand


; handler for ????, Spell ID #$D1: Berserk


Yoichi's Bow can be used in combat to cast Berserk on the party, if that's what the ???? is referring to.

I'm pretty sure the spell jump table also covers items, weapons, and enemy attacks, I just haven't hooked them all up yet. Still plenty of work to do!

[redmagejoe]

When I've got some downtime, I'll compare the versions of the level up routine rework, and then see what can be done with spell code to make room for changes and fixes as per your suggestion.

I'm going to compare and play with the Level Up Routine update first to see that we get rid of that nasty graphics bug. After that, I'll start working with the spell code to clean it up and see if Life and Sap and Swap can't be toyed with. If enough space can be made with abstraction, it may give us more space overall for down the line when we need as much space for enhancements as possible, not just the spell bug fixes.

Yep, just that one change seemed to fix everything. Uploaded it to link in first post. Hopefully we're done with that Level Up Routine Rework patch for real this time. I'll look at the spell code tomorrow. I will also consider changing the way the Chaos Rush fix is handled after this project finishes and v2.0 of Chaos Rush becomes my priority. I will admit I'm not happy with the workaround I used, but for now at least it's workable. I don't like to take shortcuts if I don't have to. Hopefully you're right and it's a very easy fix.

Code Select Expand


; handler for Spell ID #$E3: Swap
; indirect control flow target (via $BEAE)
0x03391B|$0C:$B90B:20 23 BE JSR $BE23  ; calculate number of spell attack successes in $52-$53 and number of spell resist successes in $54-$55
0x03391E|$0C:$B90E:20 D0 B5 JSR $B5D0  ; $52-$53 -= $54-$55, i.e. net spell successes
0x033921|$0C:$B911:90 50    BCC $B963  ; negative successes => Ineffective
0x033923|$0C:$B913:F0 4E    BEQ $B963  ; 0 successes => Ineffective
0x033925|$0C:$B915:A0 0A    LDY #$0A    ; Battle stat offset for Current HP low byte
0x033927|$0C:$B917:20 66 B9 JSR $B966  ; given a (current HP or MP) stat offset in Y, swap actor's and target's Y and Y + 1 stats
0x03392A|$0C:$B91A:A0 0C    LDY #$0C    ; Battle stat offset for Current MP low byte
0x03392C|$0C:$B91C:20 66 B9 JSR $B966  ; given a (current HP or MP) stat offset in Y, swap actor's and target's Y and Y + 1 stats
; cap actor's and target's current HP at their max HP
0x03392F|$0C:$B91F:A9 0A    LDA #$0A    ; Battle stat offset for Current HP low byte
0x033931|$0C:$B921:85 5C    STA $5C   
0x033933|$0C:$B923:A9 0E    LDA #$0E    ; Battle stat offset for Max HP low byte
0x033935|$0C:$B925:85 5D    STA $5D   
; copy actor's battle stats pointer to $5E-$5F
0x033937|$0C:$B927:A5 9F    LDA $9F    ; pointer to actor's battle stats
0x033939|$0C:$B929:85 5E    STA $5E   
0x03393B|$0C:$B92B:A5 A0    LDA $A0   
0x03393D|$0C:$B92D:85 5F    STA $5F   
0x03393F|$0C:$B92F:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
; copy target's battle stats pointer to $5E-$5F
0x033942|$0C:$B932:A5 A1    LDA $A1    ; pointer to target's battle stats
0x033944|$0C:$B934:85 5E    STA $5E   
0x033946|$0C:$B936:A5 A2    LDA $A2   
0x033948|$0C:$B938:85 5F    STA $5F   
0x03394A|$0C:$B93A:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
; cap actor's and target's current MP at their max MP
0x03394D|$0C:$B93D:A9 0C    LDA #$0C    ; Battle stat offset for Current MP low byte
0x03394F|$0C:$B93F:85 5C    STA $5C   
0x033951|$0C:$B941:A9 10    LDA #$10    ; Battle stat offset for Max MP low byte
0x033953|$0C:$B943:85 5D    STA $5D   
; copy actor's battle stats pointer to $5E-$5F
0x033955|$0C:$B945:A5 9F    LDA $9F    ; pointer to actor's battle stats
0x033957|$0C:$B947:85 5E    STA $5E   
0x033959|$0C:$B949:A5 A0    LDA $A0   
0x03395B|$0C:$B94B:85 5F    STA $5F   
0x03395D|$0C:$B94D:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
; copy target's battle stats pointer to $5E-$5F
0x033960|$0C:$B950:A5 A1    LDA $A1    ; pointer to target's battle stats
0x033962|$0C:$B952:85 5E    STA $5E   
0x033964|$0C:$B954:A5 A2    LDA $A2   
0x033966|$0C:$B956:85 5F    STA $5F   
0x033968|$0C:$B958:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
0x03396B|$0C:$B95B:A9 37    LDA #$37    ; Battle Message ID #$37: 'Swapped stats'
0x03396D|$0C:$B95D:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x033970|$0C:$B960:4C 7E BE JMP $BE7E  ; set $4A-$4B to 0


Took me a couple read-throughs of your explanation, but I think I see what you mean. You're saying that I could simply remove B945 up to B94B so that it JSRs immediately after B943, and then replace the following (redundant) LDAs and STAs with the removed ones. In other words...

Code Select Expand


; copy target's battle stats pointer to $5E-$5F
0x03394A|$0C:$B93A:A5 A1    LDA $A1    ; pointer to target's battle stats
0x03394C|$0C:$B93C:85 5E    STA $5E   
0x03394E|$0C:$B93E:A5 A2    LDA $A2   
0x033950|$0C:$B940:85 5F    STA $5F   
0x033952|$0C:$B942:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
; cap actor's and target's current MP at their max MP
0x033955|$0C:$B945:A9 0C    LDA #$0C    ; Battle stat offset for Current MP low byte
0x033957|$0C:$B947:85 5C    STA $5C   
0x033959|$0C:$B949:A9 10    LDA #$10    ; Battle stat offset for Max MP low byte
0x03395B|$0C:$B94B:85 5D    STA $5D
0x03395D|$0C:$B94D:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
; copy actor's battle stats pointer to $5E-$5F
0x033960|$0C:$B950:A5 9F    LDA $9F    ; pointer to actor's battle stats
0x033962|$0C:$B952:85 5E    STA $5E
0x033964|$0C:$B954:A5 A0    LDA $A0
0x033966|$0C:$B956:85 5F    STA $5F
0x033968|$0C:$B958:20 83 B9 JSR $B983  ; given an offset to low byte of current HP/MP in $5C, an offset to low byte of max HP/MP in $5D, and a pointer to some entity's battle stats in $5E-$5F, cap entity's current HP/MP at their max HP/MP
0x03396B|$0C:$B95B:A9 37    LDA #$37    ; Battle Message ID #$37: 'Swapped stats'
0x03396D|$0C:$B95D:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x033970|$0C:$B960:4C 7E BE JMP $BE7E  ; set $4A-$4B to 0


Addresses changed to reflect if I were to shift it down the full 8 bytes. I'd like to try and future-proof by only moving each section as much as necessary, so more likely, assuming it would only shift 6 bytes until I can find other spell code that needs to be cleaned up.

EDIT: Urgh, I just noticed that second "BUG" comment in Sap for the quotient, so I actually need another 4 bytes for and LDA $05 and STA ($A1),Y. That's 2 more bytes than I get from the change to Swap. Guess I might have to do a little more cleaning of spell code than I'd initially intended to fix Sap. Life can be fixed right now, but I'd rather have them both in the same patch since the same area of code is being modified.

[abw]

Disassembly updated with more comments, more bugs noticed:

• The logic for determining whether a monster will run or not calculates (total party HP - total monster HP) >> 5, but then ignores the high byte of the result and the possible carry from adding the monster's fear level.
• Status-curing items don't set the variable containing the target's previous ailments when calling the code for determining which battle message to display, resulting in an incorrect message being displayed (e.g. if you suffer from both Amnesia and Curse, curing Curse in battle with a Cross will display the message for curing Amnesia.
• Cyclone/Quake/Tsunami should be resisted by certain monster families, but the branching code checks the wrong processor flag (C instead of Z), which means family resistance is broken...
• ... but that bug is cancelled out by another bug where after calculating the target's number of successful resists, the result is never used, which means resistance for these spells is just totally broken.
• Imbibe decreases the target's evasion % by 1, but there is no check for underflow, so you could end up going from 0 to 255 % evasion; having 255 evasion probably breaks other logic elsewhere.
• In the dialogue immediately before and after fighting the boss monster when retaking Fynn castle, there's a graphics glitch after closing the dialogue boxes that causes Hilda and Gordon to appear on the thrones for a few frames.

Code Select Expand


; handler for ????, Spell ID #$D1: Berserk


Yoichi's Bow can be used in combat to cast Berserk on the party, if that's what the ???? is referring to.

It turns out that extra ??? was for Imbibe calling the Berserk handler.

I'd like to try and future-proof by only moving each section as much as necessary,

I like the intent, but the more bug fixes we make, the more things are going to shift, and the deeper we look into this code, the more bugs we're going to find and want to fix, so this could be tough to achieve.

In other news, I finally got back to looking at the weapon special effects, and have updated it to include capping the drain amount at the target's current HP/MP. I think making sure the battle poses get updated in a timely manner is the only thing left to incorporate, but that's an issue that applies to more than just weapon special effects.

I also uploaded a rewrite of the Ultima code to make it more awesome .

[redmagejoe]

I think that I'm going to have to just go ahead and remove the strike-outs for fixed bugs in the first post with all these new bugs coming in. I'm constantly skirting the character limit of posts, and while I'd like people to be able to review in post what the bugs were and what's been fixed, I need the room. I can't wait to see what you've done with Ultima. Would you say the Weapon Special Effects patch is ready for stable? I"m assuming that the battle pose update isn't tied directly to special weapons code, yeah?

[pppoe]

 I also wrote an automatic combat program for this game. Simplify the operation in the game, improve the quality of the system, send me a message, send the game source file 3667650@qq.com

[abw]

I think that I'm going to have to just go ahead and remove the strike-outs for fixed bugs in the first post with all these new bugs coming in. I'm constantly skirting the character limit of posts, and while I'd like people to be able to review in post what the bugs were and what's been fixed, I need the room.

I can help with that a little - I added my SRAM map to https://datacrystal.romhacking.net/wiki/Final_Fantasy_II:RAM_map; it was based on Jiggers' but has since been corrected and expanded, so you could link to that instead to save a few thousand characters.

I can't wait to see what you've done with Ultima.

My Ultima patch might actually be too OP - at 99 Spirit, < 49% spell success % penalty, and all skills level 16, it averages 24,006 damage and tops out at 46,464 damage, which seems excessive considering the strongest enemy in the game only has 10,000 HP. On the other hand, if you've spent however many hours it would take to max out all your skills, maybe being able to end any battle in one move should be your reward .

Would you say the Weapon Special Effects patch is ready for stable? I"m assuming that the battle pose update isn't tied directly to special weapons code, yeah?

It might need to be updated once we get around to tracking down the battle pose stuff, but I'd say it's ready for now.

[redmagejoe]

Yikes, I mean... it's better than it currently is, though I think maybe we should try to figure out the formula the remake uses. Specifically, the version that you use in the normal game, since apparently Minwu's version in Rebirth of Souls is significantly more powerful and has a different formula.

I'll go ahead and add the Weapon Special Effects fix to stable then, update the first pose, and modify the SRAM listing. I appreciate you taking care of that. Sadly, I already removed those items and I can't be assed to rewrite it all. I think it looks more clean now anyway, and as we make patches I'm adding a comprehensive explanation in a text file to be used as a readme someday.

Take Protect/Berserk for example: aside from the constants for the offset of the stat to buff and the battle message to display, they're basically the exact same routine duplicated.

I'm comparing them side-by-side, but there's enough differences in instructions that I'd have to see if branches alone can save enough space to justify rewriting one or both.

Code Select Expand


0x033806|$0C:$B7F6:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x033809|$0C:$B7F9:A9 E6    LDA #$E6    ; Battle Message ID #$66: ' up!'
0x03380B|$0C:$B7FB:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x03380E|$0C:$B7FE:4C 7E BE JMP $BE7E  ; set $4A-$4B to 0

...

0x033A78|$0C:$BA68:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x033A7B|$0C:$BA6B:A9 E6    LDA #$E6    ; Battle Message ID #$66: ' up!'
0x033A7D|$0C:$BA6D:20 92 BF JSR $BF92  ; add battle message ID A to the list of battle messages to display
0x033A80|$0C:$BA70:4C 7E BE JMP $BE7E  ; set $4A-$4B to 0


At the very least, these parts are identical and I can save 8 bytes by replacing the JSR in the first block (Protect) with a JMP to $BA68 since it should more or less be the exact same control flow. This gives me 8 more bytes to work with and comes before Life, Sap, and Swap, so I can shift all of that code up those 8 bytes, have the space I need, and 6 leftover bytes for expanding. At some point will need that and probably then some for preventing Protect/Berserk bonus from overflowing. I'll work with all of this for a bit with NOP padding before I start committing to moving anything around, need to see how I can consolidate spell code before I shift everything and change pointers to spell handlers.

Code Select Expand


; handler for Battle Commands #$15: Cast Spell ID #$D4: Cure and #$29: Cast Heal
; indirect control flow target (via $BE8E)
0x0335A5|$0C:$B595:A0 15    LDY #$15    ; Battle stat offset for Monster Family (80:Undead, 40:Werebeast, 20: Dragon, 10:Spellcaster, 08:Giant, 04:Earth, 02:Aquatic, 01:Magic Beast)
0x0335A7|$0C:$B597:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x0335A9|$0C:$B599:29 80    AND #$80    ; Undead
0x0335AB|$0C:$B59B:D0 06    BNE $B5A3  ; attacked Undead with Cure; why not call $B571 directly?
0x0335AD|$0C:$B59D:20 FB BC JSR $BCFB  ; heal target's HP by unresistable spell damage amount, set $7CB0 to #$0B, queue battle message, set $4A-$4B to 0
0x0335B0|$0C:$B5A0:4C A6 B5 JMP $B5A6  ; RTS; useless op; above JSR should just JMP instead

; attacked Undead with Cure; why not call $B571 directly?
; control flow target (from $B59B)
0x0335B3|$0C:$B5A3:20 52 BD JSR $BD52  ; calculate and inflict resistable spell damage; routine is identical to $B571
; control flow target (from $B5A0)
0x0335B6|$0C:$B5A6:60      RTS        ; useless op; above JSR should just JMP instead


I'm assuming based on your comments here that the code should look more like...

Code Select Expand


; handler for Battle Commands #$15: Cast Spell ID #$D4: Cure and #$29: Cast Heal
; indirect control flow target (via $BE8E)
0x0335A5|$0C:$B595:A0 15    LDY #$15    ; Battle stat offset for Monster Family (80:Undead, 40:Werebeast, 20: Dragon, 10:Spellcaster, 08:Giant, 04:Earth, 02:Aquatic, 01:Magic Beast)
0x0335A7|$0C:$B597:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x0335A9|$0C:$B599:29 80    AND #$80    ; Undead
0x0335AB|$0C:$B59B:D0 03    BNE $B5A0  ; attacked Undead with Cure; why not call $B571 directly?
0x0335AD|$0C:$B59D:4C FB BC JMP $BCFB  ; heal target's HP by unresistable spell damage amount, set $7CB0 to #$0B, queue battle message, set $4A-$4B to 0
0x0335B0|$0C:$B5A0:4C 71 B5 JMP $B571  ; calculate and inflict resistable spell damage
0x0335B3|$0C:$B5A3:EA NOP
0x0335B4|$0C:$B5A4:EA NOP
0x0335B5|$0C:$B5A5:EA NOP
0x0335B6|$0C:$B5A6:EA NOP


Also, I'm assuming that it's safe to remove (pad currently) these 8 bytes and simply replace all control flow redirects to it to $B571 instead:

Code Select Expand


; calculate and inflict resistable spell damage; routine is identical to $B571
; control flow target (from $B5A3, $BB17)
0x033D62|$0C:$BD52:20 7B BD JSR $BD7B  ; calculate resistable spell damage in $4A-$4B
0x033D65|$0C:$BD55:A0 0A    LDY #$0A    ; Battle stat offset for Current HP low byte
0x033D67|$0C:$BD57:4C 79 B5 JMP $B579  ; subtract amount in $4A-$4B from ($A1),Y-($A1),(Y+1), set to 0 if negative; called with Y = #$0A to reduce target's current HP


I am curious, is there a reason this line even exists? It doesn't seem like it's necessary as it's literally jumping down to the next line, unless there's some implicit side effects of a JMP instruction I'm not understanding. Otherwise, couldn't we take these 3 bytes back?

Code Select Expand


0x033586|$0C:$B576:4C 79 B5 JMP $B579


Also, is it safe to assume that your 16 "useless OP" comments means I can NOP out those bytes for when we do code-shifting and consolidation in the future? I'm also aiming once more to move as little code as possible until we've fixed all bugs, make a Bug Fix Only patch to be used as a base for any future hackers for Final Fantasy II, and then after that is stable and ready and safely backed up in an IPS file, begin the drastic overhaul of code for the rest of Restored's enhancements.

Here's my trimming in progress, so you can see my thought pattern before any addresses are changed: https://drive.google.com/open?id=1GcTbDCK9J9YCJEJabSCSA_RN1Ku5z4jD

Code Select Expand


0x0337EF|$0C:$B7DF:20 98 FC JSR $FC98  ; 16-bit multiplication (little endian): $00-$01 * $02-$03, product stored in $04-$07; leaves X at #$00, C clear, $00-$01 and Y not changed
0x0337F2|$0C:$B7E2:A0 02    LDY #$02    ; Battle stat offset for Defense
0x0337F4|$0C:$B7E4:18      CLC        ; add product to target's Defense; BUG: if product > 255, high byte is not checked, so if you score +256, you actually get +0 :(
0x0337F5|$0C:$B7E5:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x0337F7|$0C:$B7E7:65 04    ADC $04
0x0337F9|$0C:$B7E9:91 A1    STA ($A1),Y ; pointer to target's battle stats
0x0337FB|$0C:$B7EB:90 07    BCC $B7F4  ; cap Defence at 255; BUG: incorrectly displays 'Ineffective' when buffing from <= 255 to > 255
0x0337FD|$0C:$B7ED:A9 FF    LDA #$FF
0x0337FF|$0C:$B7EF:91 A1    STA ($A1),Y ; pointer to target's battle stats


Was trying to think of the best way to address this, and my current spitball is to have a BCS before the CLC? I mean, is it not safe to assume that if the high byte was set with anything other than 0, that Carry is Set, and thus it means that the bonus you're getting is >255? The fix for Protect and Berserk could be as simple as saying if we return from FC98 with a carry, then jump straight to LDA #$FF, right? Or am I misinterpreting the math? Should I be doing a LDA $05, CMP #$00, BEQ instead?

[abw]

Yikes, I mean... it's better than it currently is, though I think maybe we should try to figure out the formula the remake uses. Specifically, the version that you use in the normal game, since apparently Minwu's version in Rebirth of Souls is significantly more powerful and has a different formula.

Even counting skill levels from 0, 24 skills * 15 skill level + 100 base power + 24 power from Spirit = 484 base damage per success, which works out to an average of 750.2 total damage per success; assuming a 100% success rate, a level 16 spell gets 32 successes, so that's 24006 damage. Maybe just divide spell power by 2? Do the remakes use the same enemy HP or incorporate any magic damage defence like the existing physical damage defence?

I'm comparing them side-by-side, but there's enough differences in instructions that I'd have to see if branches alone can save enough space to justify rewriting one or both.

I was thinking more along the lines of this, which is still 24 bytes shorter than the original even after fixing 3 bugs:

Code Select Expand


; The Protect and Berserk spells are essentially the same code operating on different stats; this patch combines both routines to save space and fixes the following bugs:
; 1) the original code calculates the product of Spell Power * Spell Successes but then ignores the high byte, so scoring e.g. +256 actually gets you +0;
; 2) the original code displays the "Ineffective" battle message whenever the new stat value > 255, which is incorrect when the old stat value was < 255;
; 3) the original code does not display the "Ineffective" battle message in cases where the stat's value did not change.

; from $0C:$BE9E
Protect_handler:
LDA #$02 ; Battle stat offset for Defense
STA $5E
LDA #$5C ; Battle Message ID #$5C: 'Def.'
STA $5F
BNE Protect_Berserk_common
; from $0C:$BEB4
Berserk_handler:
LDA #$1A ; Battle stat offset for Primary Hand Attack Power
STA $5E
LDA #$5B ; Battle Message ID #$5B: 'Att.'
STA $5F
Protect_Berserk_common:
JSR $BDE9 ; calculate number of spell attack attempts in $46 and spell attack successes in $48
LDY #$27 ; Battle stat offset for (modified) Spell Power
LDA ($9F),Y ; pointer to actor's battle stats
; calculate spell power * spell successes
STA $00
LDA #$00
STA $01
LDA $48
STA $02
LDA $49
STA $03
JSR $FC98 ; 16-bit multiplication (little endian): $00-$01 * $02-$03, product stored in $04-$07; leaves X at #$00, C clear, $00-$01 and Y not changed
LDY $5E ; Battle stat offset for stat to raise
LDA ($A1),Y ; pointer to target's battle stats
STA $5E ; save old stat value
LDA $05 ; high byte of product
BNE .cap ; if product > 255, cap stat at 255
; add product to target's stat; we know C is clear thanks to the CMP #$FF
LDA ($A1),Y ; pointer to target's battle stats
ADC $04 ; low byte of product
BCC .write_stat ; if new stat > 255, cap stat at 255
.cap:
LDA #$FF
.write_stat:
STA ($A1),Y ; pointer to target's battle stats
CMP $5E
BEQ .ineffective ; if new stat value == old stat value, display "Ineffective"
LDA $5F ; Battle Message ID for stat to raise
JSR $BF92 ; add battle message ID A to the list of battle messages to display
LDA #$E6 ; Battle Message ID #$66: ' up!'
JSR $BF92 ; add battle message ID A to the list of battle messages to display
JMP $BE7E ; set $4A-$4B to 0
.ineffective:
JMP $BE73 ; clear bit 6 of $28, queue Battle Message ID #$14: 'Ineffective', set $4A-$4B to 0

but if you want to keep them in their original locations, there's enough free space to toss in some JMPs between the two regions.

I'm assuming based on your comments here that the code should look more like...

Code Select Expand


; handler for Battle Commands #$15: Cast Spell ID #$D4: Cure and #$29: Cast Heal
; indirect control flow target (via $BE8E)
0x0335A5|$0C:$B595:A0 15    LDY #$15    ; Battle stat offset for Monster Family (80:Undead, 40:Werebeast, 20: Dragon, 10:Spellcaster, 08:Giant, 04:Earth, 02:Aquatic, 01:Magic Beast)
0x0335A7|$0C:$B597:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
0x0335A9|$0C:$B599:29 80    AND #$80    ; Undead
0x0335AB|$0C:$B59B:D0 03    BNE $B5A0  ; attacked Undead with Cure; why not call $B571 directly?
0x0335AD|$0C:$B59D:4C FB BC JMP $BCFB  ; heal target's HP by unresistable spell damage amount, set $7CB0 to #$0B, queue battle message, set $4A-$4B to 0
0x0335B0|$0C:$B5A0:4C 71 B5 JMP $B571  ; calculate and inflict resistable spell damage
0x0335B3|$0C:$B5A3:EA NOP
0x0335B4|$0C:$B5A4:EA NOP
0x0335B5|$0C:$B5A5:EA NOP
0x0335B6|$0C:$B5A6:EA NOP


You can also get rid of the JMP $B571 if you just BNE $B571 directly instead of BNE $B5A0 -> JMP $B571.

Also, I'm assuming that it's safe to remove (pad currently) these 8 bytes and simply replace all control flow redirects to it to $B571 instead:

Code Select Expand


; calculate and inflict resistable spell damage; routine is identical to $B571
; control flow target (from $B5A3, $BB17)
0x033D62|$0C:$BD52:20 7B BD JSR $BD7B  ; calculate resistable spell damage in $4A-$4B
0x033D65|$0C:$BD55:A0 0A    LDY #$0A    ; Battle stat offset for Current HP low byte
0x033D67|$0C:$BD57:4C 79 B5 JMP $B579  ; subtract amount in $4A-$4B from ($A1),Y-($A1),(Y+1), set to 0 if negative; called with Y = #$0A to reduce target's current HP


Yup, $0C:$BD52 is a complete waste of space.

I am curious, is there a reason this line even exists? It doesn't seem like it's necessary as it's literally jumping down to the next line, unless there's some implicit side effects of a JMP instruction I'm not understanding. Otherwise, couldn't we take these 3 bytes back?

Code Select Expand


0x033586|$0C:$B576:4C 79 B5 JMP $B579


That one's also useless, though only because $B579 (which is called from a lot of places) just happens to be next address.

Also, is it safe to assume that your 16 "useless OP" comments means I can NOP out those bytes for when we do code-shifting and consolidation in the future?

Sometimes NOP is fine, sometimes nearby code might need to be tweaked. It depends on the situation, but the right thing to do should be fairly obvious in each case - I was more concerned with figuring out what the code was doing than with looking at how inefficient it was, so those "useless op" notes tend to be about superficial things.

The fix for Protect and Berserk could be as simple as saying if we return from FC98 with a carry, then jump straight to LDA #$FF, right? Or am I misinterpreting the math? Should I be doing a LDA $05, CMP #$00, BEQ instead?

$FC98 always leaves C clear, so you will have to check $05, but LDA updates Z for you, so you don't need an explicit CMP #$00 and can just BEQ.

[redmagejoe]

I'll leave the "useless" alone for now and it can be something we review later when we need to code shift. For now, I've taken back the three bytes for the useless JMP, and as per your suggestion, removed the JMP and turned the BNE 03 into a BNE D4 to go back up to $B571 to give us another three bytes. I'm going to try that rewrite like you suggested, as 24 bytes is extremely appealing. The rewrite should be functionally the same with no surprise side effects, yes?

I'm going to try and study the remake's Ultima before I sit down and tackle that. As for the Protect/Berserk >255 fix... well, I guess your rewrite already addresses that, from a cursory glance? The $A1 change to $9E that fixes Protect not working on other party members I assume is also in, and... what was the third bug again? Been working on this so long I'm forgetting what bugs we fixed already. 

So original Berserk code is 51 bytes, and original Protect code is 55 bytes (total of 106 bytes). Your rewritten code is 82 bytes. That doesn't quite fit in the original space for either of them, but considering the use of branch demands that they be within a certain range of each other, I think it would be best to move Berserk and Protect right up into the same space and shift everything else down. A lot of pointers are going to be rewritten for this, but I think it will be worth the effort in the long run. I'll get to work and update the txt file in the link as soon as I can. This will become quite involved and will involve rewriting a large portion of spell handler code, so I'm going to grab the three spell-related fixes, extract the relevant instructions and set them aside to be incorporated into another comprehensive patch. If you spot any other places where code can be made more efficient that I might overlook, let me know.

EDIT: Updated txt, still WIP. I haven't worked in the redone Protect/Berserk code yet as I'd like to look at other places that spell code can be cleaned up before we settle on shifting everything. For example, the excerpt of code now contains the Wall code, the Drain/Osmose code (let's look at that battle pose bug, shall we?), the Cyclone/Quake/Tsunami code, and will soon contain the Imbibe code. This seems like a good opportunity to rewrite this entire bank just about and make sure the entire spell system is bug free and efficient.

[abw]

The rewrite should be functionally the same with no surprise side effects, yes?

That's the plan, yes, though I haven't tested any of it yet .

The $A1 change to $9E that fixes Protect not working on other party members I assume is also in, and... what was the third bug again? Been working on this so long I'm forgetting what bugs we fixed already. 

Oh, right, I forgot to list the "Protect only working on the caster" bug, so that's actually 4 bugs fixed then. The other 3 I listed in comments:

• the original code calculates the product of Spell Power * Spell Successes but then ignores the high byte, so scoring e.g. +256 actually gets you +0;
• the original code displays the "Ineffective" battle message whenever the new stat value > 255, which is incorrect when the old stat value was < 255;
• the original code does not display the "Ineffective" battle message in cases where the stat's value did not change.

I also forgot to mention that I finally got around to fixing up the reassemblable version of the disassembly, so for a large-scale rewrite like this is shaping up to be, it's probably better to work from that than the non-assemblable version.

And since I'm up to that point in my playthrough now, I noticed another place where calculated stats don't get updated after gaining base stats: the stat boosts from the Mysidian Tower Orbs!

[redmagejoe]

That's probably a good place to insert another stat refresh, though I'll look at that alongside the optional patch after this big project. I'm going to take a look at the reassemble version and add that bug to the list. Also I meant 9F, not 9E.

Code Select Expand


0x033825|$0C:$B815:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
; control flow target (from $B81B)
0x033827|$0C:$B817:20 0E 90 JSR $900E  ; clear the X'th bit (0-7) of A
0x03382A|$0C:$B81A:CA      DEX       
0x03382B|$0C:$B81B:10 FA    BPL $B817 
; BUG: STA ($A1),Y needs to go here! Without this, the spell has no effect!
0x03382D|$0C:$B81D:A2 00    LDX #$00   


My Dispel fix actually has the STA ($A1),Y above the DEX. Should it be after the branch instead? I'm trying to remember why I thought it needed to go there... I moved the transcribe WIP down to directly above the Dispel code so you can review if I did something wonky with it. Updated the txt again, this is as far as I'm comfortable shifting the mock-up until I look at the code a bit more closely and see where I can find inefficiencies or bugs that will need space. I moved only what seemed to be as clean as it could be thus far.



For the sake of as accurate an Ultima as possible to the remade version, I am doing some testing. First tests are with a 99 Spirit Firion (equipment doesn't matter as there's no stat penalties on equipment in remakes, to my knowledge) with 8/8 weapons at 16, and 16/16 spells at 16 including Ultima. I will use cheats to adjust levels as necessary and post damage values on the lowest magic resist enemies available (1-50%). Thankfully Kashuan Keep still has pre-Cyclone enemies to test on.

Code Select Expand


Weapons 8x 16 - Spells 16x 16
7867, 7876, 7899, 7918, 7934, 7941, 7969, 9999 (crit?)

Weapons 8x 16 - Spells 16x 1
244, 288, 299, 319, 325, 355

Weapons 8x 1 - Spells 16x 1
138, 142, 152, 184, 230

Weapons 8x 0 - Spells 16x 0
121, 122, 126, 147, 155, 157, 168

Weapons 7x 0, 1x 16 - Spells 15x 0, Ultima 2
157, 172, 180, 188, 195, 248


First note: Ultima completely ignores Spirit, Intellect, Magic Power. It seems to only be affected by its own level and skill levels, as stats made no difference between 0 and 99. Otherwise, it's very difficult to tell exactly how it's behaving without digging into the source of the game itself (which is a whole other Pandora's box) I would gladly welcome the interest of anyone who might want to do some extensive testing of Ultima on the PSP version, since it seems like it could become a very time-consuming process to reverse engineer. I have available the cheats file and a save file for all necessary testing, but I don't want to get too distracted from the bug-fixing at the moment.

[abw]

My Dispel fix actually has the STA ($A1),Y above the DEX. Should it be after the branch instead?

Both places will work, but you save cycles by only writing the final result once after the loop finishes instead of writing all the intermediate results inside the loop.

For the sake of as accurate an Ultima as possible to the remade version, I am doing some testing.

On the NES, spell damage per success is calculated the same way as physical damage per success, except the target's defense value is always 0. So each success deals (spell power) + random([0..(spell power)]) with a 5% chance to critical hit, which adds (spell power) to the total; that works out to anywhere between (spell power) and 3 * (spell power) with an average value of (spell power) * 1.55.

Spell power gets modified by the caster's Intellect/Spirit before jumping to the individual spell handlers, so if we want to undo that and use Ultima's base power, we'll have to fetch it again.

I suppose no matter what we do, Ultima's damage should get capped at 9999 anyway.

[redmagejoe]

Yeah, I'm going to set aside Ultima testing until everything else just about in the spell block is ready to go. It seems like it may become more complicated, and it would be nice if I could find someone who wanted to do some exhaustive testing on it on PSP.

So having revised my Dispel fix as I'm polishing up spell code, I am assuming that the way to best fix this is to A) remove the first BPL, B) change the LDX #$07 to LDX #$08, and C) add the STA ($A1),Y, which for the sake of saving cycles will be moved to after the BPL that jumps up two instructions.

Code Select Expand


0x033817 10 02 30 30 E0 08 90 02 A2 07 86 52 A0 05 B1 A1 20 0E 90 CA 10 FA
>>>
0x033817       30 32 E0 08 90 02 A2 08 86 52 A0 05 B1 A1 20 0E 90 CA 10 F8 91 A1


From

Code Select Expand


0x033817|$0C:$B807:10 02    BPL $B80B 
0x033819|$0C:$B809:30 30    BMI $B83B  ; 0 successes => Ineffective
; control flow target (from $B807)
0x03381B|$0C:$B80B:E0 08    CPX #$08   
0x03381D|$0C:$B80D:90 02    BCC $B811  ; cap X at 7
0x03381F|$0C:$B80F:A2 07    LDX #$07   
; control flow target (from $B80D)
0x033821|$0C:$B811:86 52    STX $52   
0x033823|$0C:$B813:A0 05    LDY #$05    ; Battle stat offset for Armor Resistance bits (80:Ice, 40:Body, 20:Poison, 10:Death, 08:Lightning, 04:Mind, 02:Fire, 01:Matter)
0x033825|$0C:$B815:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
; control flow target (from $B81B)
0x033827|$0C:$B817:20 0E 90 JSR $900E  ; clear the X'th bit (0-7) of A
0x03382A|$0C:$B81A:CA      DEX       
0x03382B|$0C:$B81B:10 FA    BPL $B817


to

Code Select Expand


0x033817|$0C:$B807:30 32    BMI $B83B  ; 0 successes => Ineffective
0x033819|$0C:$B809:E0 08    CPX #$08   
0x03381B|$0C:$B80B:90 02    BCC $B80F  ; cap X at 8
0x03381D|$0C:$B80D:A2 08    LDX #$08   
; control flow target (from $B80B)
0x03381F|$0C:$B80F:86 52    STX $52   
0x033821|$0C:$B811:A0 05    LDY #$05    ; Battle stat offset for Armor Resistance bits (80:Ice, 40:Body, 20:Poison, 10:Death, 08:Lightning, 04:Mind, 02:Fire, 01:Matter)
0x033823|$0C:$B813:B1 A1    LDA ($A1),Y ; pointer to target's battle stats
; control flow target (from $B819)
0x033825|$0C:$B815:20 0E 90 JSR $900E  ; clear the X'th bit (0-8) of A
0x033828|$0C:$B818:CA      DEX       
0x033829|$0C:$B819:10 FA    BPL $B815
0x03382B|$0C:$B81B:91 A1    STA ($A1),Y


[abw]

So having revised my Dispel fix as I'm polishing up spell code, I am assuming that the way to best fix this is to A) remove the first BPL, B) change the LDX #$07 to LDX #$08, and C) add the STA ($A1),Y, which for the sake of saving cycles will be moved to after the BPL that jumps up two instructions.

Yes to A and C, no to B, unless you're also going to rewrite $900E to correctly handle clearing the 9th bit of a byte .

I spent some more time commenting on code and have updated both versions of the disassembly with the results, including progress on working through the battle pose mechanics, which is tied in to status ailments and their display, which opened up a rabbit hole I didn't feel like going down at the time. So instead I rewrote the spell handlers to fix the following known bugs:

• Life only writes the low byte of HP to be restored on success, which may result in leaving the target dead due to 16-bit math error.
• Sap only acts on the least significant byte rather than the whole of a target's MP. Investigate and correct.
• Osmose drains MP even if the target has none. Needs to check source MP before awarding it.
• Protect and Berserk bonus may overflow before being added to Defense or Attack, resulting in low or no effect.
• Wall can be exploited by casting it on monsters to ensure that Toad, Break, Death, and Warp work on them. Wall's effects should likely be checked before any routines handling animation or other effects take place. Investigate.
• Cyclone/Quake/Tsunami should be resisted by certain monster families, but the branching code checks the wrong processor flag (C instead of Z), which means family resistance is broken but that bug is cancelled out by another bug where after calculating the target's number of successful resists, the result is never used, which means resistance for these spells is just totally broken.
• Imbibe decreases the target's evasion % by 1, but there is no check for underflow, so you could end up going from 0 to 255 % evasion; having 255 evasion probably breaks other logic elsewhere.
• Status-curing items don't set the variable containing the target's previous ailments when calling the code for determining which battle message to display, resulting in an incorrect message being displayed (e.g. if you suffer from both Amnesia and Curse, curing Curse in battle with a Cross will display the message for curing Amnesia.

and the following further bugs I came across while doing so:

• Basuna always clears the Critical HP flag
• Basuna always cures Poison, even with 0 successes
• Slow and Fear are slightly too powerful, only failing with negative net successes instead of also failing with 0 net successes
• Fear calculates spell power * net successes, but does not check the high byte of the product

as well as incorporating the earlier Level 8 Aura / Barrier Fix, Dispel Fix, and Protect Fix. Testing all of that is going to be fun .

[redmagejoe]

That is... a LOT of changes. Wish I'd known you were tackling some of these before I got halfway through my fixes.

Nonetheless, I am once again in awe of your efforts. I, uh, don't suppose you have your ASM for it handy? I'd really like to try and familiarize myself with the changes so I can understand what the instructions are doing. Not that I haven't transcribed hex to code before, but if I can avoid it...

And ooh, 54 bytes left over at the end of everything? That's a LOT of nice juicy space! We'll probably need at least some of it for the Ultima fix, once we figure out what specifically it should be doing. I suppose since you went to all the trouble to rewrite the code, I could go back to testing Ultima in Anniversary and see if I can't narrow down its behavior...

The wild fluctuation in damage makes it very difficult to pin down differences. I am convinced though that stats play absolutely no role in damage. Attacking only Leg Eaters over and over again, damage ranges from 111 to 143, with the odd 164 and 173, and a 244 what I assume is a crit. All with Ultima 1, everything else at 0. Weapon Levels do appear to affect the damage as well as spell levels, though by how much I cannot quite tell. Certainly not enough to make a patch with. I don't know how shy of running this through some sort of simulation that does over 1000 trials we could get precise enough math to make an educated guess for our patch.

[abw]

That is... a LOT of changes. Wish I'd known you were tackling some of these before I got halfway through my fixes.

Ah, sorry. Switching from documenting to coding was sort of a spur-of-the-moment thing. Once I had the reassembleable version working again, mostly all I had to do was the actual code changes, which didn't take long. Apparently I've successfully found and labelled enough of the game's pointers that making large changes in bank C does not crash the game, so that was a nice bonus . I had to do some more digging to track down the Wall bug: both the Wall spell handler and the code for preventing blocked spells from running their handlers appeared to be correct; it actually was the spell animation handlers for instant-death spells that were killing the target! Blocked spells now all use the same harmless (hopefully, anyway) animation. In any case, having independent versions makes it easier to cross-check them for errors .

Nonetheless, I am once again in awe of your efforts. I, uh, don't suppose you have your ASM for it handy? I'd really like to try and familiarize myself with the changes so I can understand what the instructions are doing. Not that I haven't transcribed hex to code before, but if I can avoid it...

ASM! Diff it against the reassembleable version to see the rewrite changes. I've also updated a couple of things after some testing and regenerated the patch, so you'll want to download the latest version.

And ooh, 54 bytes left over at the end of everything? That's a LOT of nice juicy space! We'll probably need at least some of it for the Ultima fix, once we figure out what specifically it should be doing. I suppose since you went to all the trouble to rewrite the code, I could go back to testing Ultima in Anniversary and see if I can't narrow down its behavior...

Oh, I also threw in my OP Ultima change, capped at 9999 damage, just for fun. Should be easy enough to slot in something more accurate once we have it. Another thing I should mention is that I made code changes to fix the order of the battle messages for Aura/Barrier/Dispel, so it'll be compatible with any translations.

The wild fluctuation in damage makes it very difficult to pin down differences. I am convinced though that stats play absolutely no role in damage. Attacking only Leg Eaters over and over again, damage ranges from 111 to 143, with the odd 164 and 173, and a 244 what I assume is a crit. All with Ultima 1, everything else at 0. Weapon Levels do appear to affect the damage as well as spell levels, though by how much I cannot quite tell. Certainly not enough to make a patch with. I don't know how shy of running this through some sort of simulation that does over 1000 trials we could get precise enough math to make an educated guess for our patch.

I think the NES version can do anywhere between 100 and 1116 damage depending on the RNG and your Spell Power boost, and I would not want to try to determine that damage formula observationally ! If you're able to hack it, try setting Ultima's base power to 0 (or 1 if that doesn't work); assuming the damage calculation algorithm is similar to the NES version's, that should mostly eliminate the damage fluctuations, leaving only the 5% chance per hit for a crit, which should be easy to identify within a couple of repetitions.

[redmagejoe]

It's alright, I'd rather work with a more comprehensive rewrite than my in-progress scrappy one. What, other than Wall, have you tested so far that we can cross it off our bug list? I wouldn't even begin to know how to hack the PSP version of the game. I looked at its code in the disassembler built into ppsspp, and... it's a completely different ball game there.

Should I revert the reversal of messages for Dispel and Barrier on my ChaosRush translation, then, if you've fixed the code to make it play nice with current translations?

There's a lot of fixes to test... I'll need to wait until I can devote some time away from work before I can begin tackling playtesting.



It also occurs to me that while the actual behavior of Ultima in remakes is still an enigma, there's some fairly easy math that could be used to make the final spell fairly balanced. 1 point of guaranteed damage per level in a skill. Consider 16 spells (Ultima included) and 8 weapon types, all with 16 levels. That's 24 x 16 = 384. Make the base damage of Ultima something like 100 (or 116 for the sake of nice round numbers), and you end up with 500 damage per hit, with 16 hits on a Level 16 Ultima giving ~8000 damage. Might be an easier scaling spell that way rather than having it do obscene damage and then forcing a 9999 cap.