Some changes/fixes for Sonic 1

Discussion in 'Engineering & Reverse Engineering' started by KingofHarts, Sep 4, 2012.

  1. Clownacy

    Clownacy

    Tech Members Tech Member
    775
    0
    16
    This will target the latest Git (and I mean latest, this relies on a commit I just made).

    obj45. Some may know this as the horizontal spikes-on-pole object from early Marble Zone. It's seen use in hacks that are either just trying to be creative or attempting to restore 'beta MZ'. But did you know that the object has three subtypes, and that only the default one works properly?

    Let's check them out.

    Subtype 00
    [​IMG]
    Looking good.
    Subtype 01
    [​IMG]
    Uhh.
    Subtype 02
    [​IMG]
    Well...

    Things aren't looking too good for the last two, there. So, let's see what we can do about it.

    Subtype 01
    Let's start with fixing subtype 01. The problem here are the mappings, they're unfinished (seriously, it looks like the guy was halfway through editing a copy/paste job when he gave up).

    Open up _maps/Sideways Stomper.asm, where we'll immediately see our first problem: the offset table has enough entries for six pole frames. Each pole frame is longer than the last, but the final two are the same (actually, the last three are identical, but we'll get to that later). For now, make the sixth pointer point to @pole6 (or .pole6, if you're using the AS assembler), which we'll add in a bit.

    Scroll further to find @pole5 (or .pole5, you get the idea), where you'll see our next problem. I'm not gonna explain how mappings work (SCHG FTW), but let's just say that @pole5 (and technically @pole6) is identical to @pole4 because they have the same number of 'pieces'. At least, that's what the header says. In reality, there exist 10, but the header doesn't point them out, leaving them unused. Correcting this is simple: just replace the '8' with '10' (or '$A' if you want the hex equivalent).

    Of course, we're not done yet. We may have fixed one frame, but we need to fix two, or the pole will still appear too short. Create a duplicate of @pole5, and name it @pole6. Duplicate its last two lines, and make it so each line's final value is $10 more than the previous line's, giving you this:

    Code (ASM):
    1. @pole6:     dc.b $A
    2.         dc.b $F8, 5, 0, $41, $E0
    3.         dc.b $F8, 5, 0, $41, $F0
    4.         dc.b $F8, 5, 0, $41, 0
    5.         dc.b $F8, 5, 0, $41, $10
    6.         dc.b $F8, 5, 0, $41, $20
    7.         dc.b $F8, 5, 0, $41, $30
    8.         dc.b $F8, 5, 0, $41, $40
    9.         dc.b $F8, 5, 0, $41, $50
    10.         dc.b $F8, 5, 0, $41, $60
    11.         dc.b $F8, 5, 0, $41, $70
    12.         dc.b $F8, 5, 0, $41, $80
    13.         dc.b $F8, 5, 0, $41, $90
    Don't forget to set the header to $C, to reflect the two new pieces.

    You should now be able to save and build, and...
    [​IMG]
    Goddammit.

    So, what's up this time? It seems, beyond $80, the last value (X-pos) wraps, hence why the first pieces start at $E0. But if we reduce all values by $20, making the last value <$80, then the pole will be $20 off centre. Lucky for us, we can make the pole -$20 off centre as well, negating this.

    Open _incObj/45 Sideways Stomper.asm, and take a look at SStom_Var. You can see the value 'xpos' value of 'pole'. Add $20 to it. Now we can go through the mappings, subtracting $20 from all final values of each sprite piece (remember to only do this to @poleX, leave non-pole sprites as-is).

    And with that, subtype 01 is fixed.

    Subtype 02
    Now how about subtype 02, that thing was a black hole!

    This one's much simpler. Open _incObj/45 Sideways Stomper.asm, and take a look at SStom_Len/word_B9BE. You can see that there are three entries, one for each subtype, so this can't be our problem. Instead, look at off_BAD6. Now things are looking goofy. Again, supporting the whole 'unfinished' idea, we have a useless offset table that does not account for invalid values, and lacks a pointer for subtype 02.

    To reiterate, this thing points to the same code, and only works with subtypes 00 and 01. What alternate code, if any, this would have pointed to, if it were finished, is a mystery. It seems the best we can do is just make subtype 02 point to the same code as 00 and 01. We won't be needing this pointer table, I can tell you that much. Just delete everything from (and including) SStom_Move's 'moveq' until (and including) the label 'loc_BADA'. With that, we're done.

    Conclusion
    Now, all three subtypes should be functional. What we're left with is...
    • Subtype 00 - Short length
    • Subtype 01 - Long length
    • Subtype 02 - Medium length

    The lengths are determined by SStom_Var.

    SonLVL definitions for this object are to come.
     
  2. Clownacy

    Clownacy

    Tech Members Tech Member
    775
    0
    16
    Time for a clusterfuck of a bug.

    Recently, someone pointed out that their ported S1 objects were causing crashes at bizarre times. It was found to be down to the objects displaying before they'd even initialised, which should be impossible. MarkeyJester revealed that it was, in fact, not the object's fault. It was indeed queued to display, but it wasn't queued by itself. It turned out that the previous object in that object RAM slot queued itself with DisplaySprite, then deleted itself with DeleteObject, all on the same frame. The result was that, on the next frame, a new object was loaded, and even before its code was run, it was displayed causing a crash due to the lack of a mappings pointer.

    So why doesn't this cause any problems in S1? It never crashes from this. It may not crash, but the bug's still there. It will make itself evident if you port S2/S3K's mappings format, or port the object to S2/S3K, as the invalid mappings pointer will make BuildSprites read from an odd address. Not a problem with S1 because the value loaded will be a byte, but with S2 or S3K, it'll be a word. The 68k will crash, and there's your problem.

    This is a very deep-rooted bug: many, many objects suffer from it. Even the complicated boss objects have it. It appears that whoever wrote the engine (Naka?) was well aware of the bug, but wouldn't be able to eradicate it until Sonic 2. Some objects are fixed, while others aren't. In fact, REV01 shows some objects making the transition, with obj46 having a manifestation of the bug in REV00, to being rid of it in REV01. A workaround that tries to detect invalid mappings data loaded as a result of the null pointer also exists, and a second workaround to detect null mappings was added in the Sonic 2 Betas/S2REV00. Indeed, this was evidently a bug that troubled the developer for quite some time. Leftover S1 objects in S2 also feature fixes, but what worries me is the possibility of this bug being in Sonic CD...

    After searching through every object in S1, I believe I finally have all the details on fixing this. But I ask that any strange occurrences following the correction of this bug be brought to my attention, as there easily could have been mistakes made, and instances of the bug missed.

    Lucky for us, RememberState is already fixed, so... yay?

    BUG 1

    Let's start with the easy stuff. Many objects do this:

    Code (ASM):
    1.         bsr.w   DisplaySprite
    2.         out_of_range    DeleteObject
    3.         rts
    This is bad. It should be obvious why. No matter what, DisplaySprite will always be processed, even if the object is deleted immediately afterwards. This is simplest bug to fix, as all you need to do is move the 'bsr' to after the macro. Preferably, the 'bsr' and 'rts' should be combined into a 'bra', but you should already know that.

    This can be found in
    1. _incObj/0D Signpost.asm - Signpost
    2. _incObj/1D Unused Switch.asm - Swi_ChkDel
    3. _incObj/25 & 37 Rings.asm - Ring_Animate
    4. _incObj/26 Monitor.asm - Mon_Display
    5. _incObj/32 Button.asm - But_Display
    6. _incObj/36 Spikes.asm - Spik_Display
    7. _incObj/3B Purple Rock.asm - Rock_Solid
    8. _incObj/41 Springs.asm - Springs
    9. _incObj/44 GHZ Edge Walls.asm - Edge_Display
    10. _incObj/46 MZ Bricks.asm - @chkdel
    11. _incObj/5D Fan.asm - @chkdel

    BUG 2

    Let's use obj11 as an example. This one's more complicated. The out_of_range is part of a subroutine, used in this fashion:

    Code (ASM):
    1.         bsr.w   DisplaySprite
    2.         bra.w   Bri_ChkDel
    So what we'll be doing is removing that 'bsr.w DisplaySprite', and making it a part of Bri_ChkDel, in the same fashion as in BUG 1.

    This can be found in
    1. _incObj/11 Bridge (part 1).asm - @display
    2. _incObj/11 Bridge (part 2).asm - Bri_Platform
    3. _incObj/11 Bridge (part 3).asm - Bri_ChkDel
    4. _incObj/15 Swinging Platforms (part 1).asm - Swing_Action, Swing_Action2
    5. _incObj/15 Swinging Platforms (part 2).asm - Swing_ChkDel
    6. _incObj/17 Spiked Pole Helix.asm - Hel_Action, Hel_ChkDel
    7. _incObj/18 Platforms.asm - Plat_Action, loc_7F06, Plat_ChkDel
    8. _incObj/2F MZ Large Grassy Platforms.asm - LGrass_Display, LGrass_ChkDel
    9. _incObj/45 Sideways Stomper.asm - SStom_Solid, SStom_ChkDel

    BUG 3

    This one's a little different. Using obj1A as an example, look for this:

    Code (ASM):
    1. Ledge_TimeZero:
    2.         bsr.w   ObjectFall
    3.         bsr.w   DisplaySprite
    4.         tst.b   obRender(a0)
    5.         bpl.s   Ledge_Delete
    6.         rts
    As you can see, it's similar to the first set of bugs. The fix is just as simple, move the 'bsr.w DisplaySprite' down to after the 'bpl.s Ledge_Delete'.

    This can be found in
    1. _incObj/1A Collapsing Ledge (part 1).asm - Ledge_TimeZero
    2. _incObj/3C Smashable Wall.asm - Smash_FragMove
    3. _incObj/51 Smashable Green Block.asm - Smab_Points
    4. _incObj/53 Collapsing Floors.asm - CFlo_TimeZero
    5. _incObj/55 Basaran.asm - ; unused crap

    BUG 4

    Again, similar to Bug 1. Using obj1F as an example, look for this:

    Code (ASM):
    1. Crab_BallMove:  ; Routine 8
    2.         lea (Ani_Crab).l,a1
    3.         bsr.w   AnimateSprite
    4.         bsr.w   ObjectFall
    5.         bsr.w   DisplaySprite
    6.         move.w  (v_limitbtm2).w,d0
    7.         addi.w  #$E0,d0
    8.         cmp.w   obY(a0),d0  ; has object moved below the level boundary?
    9.         bcs.s   @delete     ; if yes, branch
    10.         rts
    11.  
    12.     @delete:
    13.         bra.w   DeleteObject
    Move the 'bsr.w DisplaySprite' to under the 'bcs.s @delete'.

    This can be found in
    1. _incObj/1F Crabmeat.asm - Crab_BallMove
    2. _incObj/20 Cannonball.asm - Cbal_Display
    3. _incObj/23 Buzz Bomber Missile.asm - Msl_FromBuzz

    BUG 5

    This one's a bit complicated. It's only present in objects that delete their child objects. The bug originates from how a child, loaded into an earlier RAM slot than its parent, will queue itself to be displayed, but before it does display, its parent object deletes it. This can be rectified by forcing child objects to be in later RAM slots than its parent, ensuring that the parent deletes the child first, stopping the child's code from running and queuing itself for display.

    To my knowledge, this only affects two objects: obj15 and obj57. The fix is simple: replace the branch to FindFreeObj with one to FindNextFreeObj.

    This can be found in
    1. _incObj/15 Swinging Platforms (part 1).asm - @makechain
    2. _incObj/57 Spiked Ball and Chain.asm - @makechain

    BUG 6

    More or less the messiest manifestation of the bug. This occurs when a branch to DisplaySprite, DeleteObject or RememberState is placed outside of the routine-depandant code, like this:

    Code (ASM):
    1. PowerUp:                ; XREF: Obj_Index
    2.         moveq   #0,d0
    3.         move.b  obRoutine(a0),d0
    4.         move.w  Pow_Index(pc,d0.w),d1
    5.         jsr Pow_Index(pc,d1.w)
    6.         bra.w   DisplaySprite
    Working around this isn't the easiest thing to do. You could remove the bra.w, but you need to add it to every possible 'outcome' of the routine-specific code. This is simply too complex and prone to error. So, let's try something different. Instead, when DeleteObject is called, how about we meddle with the stack, so that bra.w is completely bypassed? It's not the neatest, and has its own error window (the results of erroneous stack-meddling can be disastrous, and difficult to track down), but I think it's the best choice we have.

    _incObj/2E Monitor Content Power-Up.asm
    Find Pow_Delete, and replace it with this:

    Code (ASM):
    1. Pow_Delete: ; Routine 4
    2.         subq.w  #1,obTimeFrame(a0)
    3.         bpl.s   @locret
    4.         addq.l  #4,sp
    5.         bra.w   DeleteObject    ; delete after half a second
    6. @locret:
    7.         rts

    _incObj/3C Smashable Wall.asm
    Find Smash_FragMove, and place a 'addq.l #4,sp' at the start.

    _incObj/3E Prison Capsule.asm
    Find Pri_EndAct, and place a 'addq.l #4,sp' before the 'jmp DeleteObject'.

    _incObj/4C & 4D Lava Geyser Maker.asm
    Find GeyserMaker, and replace the 'bra.w Geyser_ChkDel' with this:

    Code (ASM):
    1.         out_of_range    DeleteObject
    2.         rts
    Then find Geyser_ChkDel, and replace it with this:

    Code (ASM):
    1. Geyser_ChkDel:              ; XREF: GeyserMaker
    2.         out_of_range    @delete
    3.         rts
    4. @delete:
    5.         addq.l  #4,sp
    6.         bra.w   DeleteObject
    After that, find Geyser_Delete, and place a 'addq.l #4,sp' at the start.

    _incObj/51 Smashable Green Block.asm
    Find Smab_Points, and place a 'addq.l #4,sp' at the start.

    _incObj/6F SBZ Spin Platform Conveyor.asm
    Find loc_16380, and place a 'addq.l #4,sp' before the 'jmp DeleteObject'.

    BUG 6 - Bosses

    _incObj/3D Boss - Green Hill (part 2).asm
    Find BGHZ_ShipDel, and add a 'addq.l #4,sp' at the start.

    _incObj/73 Boss - Marble.asm
    Find Obj73_ShipDel, and add a 'addq.l #4,sp' at the start.

    _incObj/74 MZ Boss Fire.asm
    Find Obj74_Delete, Obj74_Delete2 and Obj74_Delete3, and add a 'addq.l #4,sp' at the start.

    _incObj/75 Boss - Spring Yard.asm
    Find Obj75_ShipDelete, and add a 'addq.l #4,sp' at the start.

    _incObj/77 Boss - Labyrinth.asm
    Find Obj77_ShipDel, and add a 'addq.l #4,sp' at the start.

    _incObj/7A Boss - Star Light.asm
    Find loc_18BE0, and replace the 'bpl.w Obj7A_Delete' with this:

    Code (ASM):
    1.         bmi.w   loc_18BE8
    2.         addq.l  #4,sp
    3.         bra.w   Obj7A_Delete

    _incObj/7B SLZ Boss Spikeball.asm
    Find Obj7B_MoveFrag, and replace the 'bpl.w Obj7A_Delete' with this:

    Code (ASM):
    1.         bmi.s   @locret
    2.         addq.l  #4,sp
    3.         bra.w   Obj7A_Delete
    4. @locret:

    _incObj/85 Boss - Final.asm
    Find loc_1A248, and add a 'addq.l #4,sp' before the 'bra.w Obj85_Delete'.[/code]

    ODDBALLS

    _incObj/13 Lava Ball Maker.asm
    This object and obj14 are linked, so the fix is rather awkward. Find LavaMaker, and replace the 'bra.w LBall_ChkDel' with this:

    Code (ASM):
    1.         out_of_range    DeleteObject
    2.         rts

    _incObj/14 Lava Ball.asm
    LBall_ChkDel, and change the 'DeleteObject' to 'LBall_Delete'. Then, go to LBall_Delete, and add a 'addq.l #4,sp' at the start.

    _incObj/23 Buzz Bomber Missile.asm
    Find Msl_Animate.

    Code (ASM):
    1. Msl_Animate:    ; Routine 2
    2.         bsr.s   Msl_ChkCancel
    3.         lea (Ani_Missile).l,a1
    4.         bsr.w   AnimateSprite
    5.         bra.w   DisplaySprite
    Here we have a case of an object being deleted and then displayed. We can't modify Msl_ChkCancel because it's used (properly) elsewhere. The better solution would just be to duplicate Msl_ChkCancel, and have it not bother with displaying if it is deleted.

    Code (ASM):
    1. Msl_Animate:    ; Routine 2
    2.         movea.l parent(a0),a1
    3.         cmpi.b  #id_ExplosionItem,0(a1) ; has Buzz Bomber been destroyed?
    4.         beq.s   Msl_Delete  ; if yes, branch
    5.         lea (Ani_Missile).l,a1
    6.         bsr.w   AnimateSprite
    7.         bra.w   DisplaySprite
    That's better.

    _incObj/29 Points.asm
    Here we'll make a bit of an optimisation. Above Poi_Index, you can see that DisplaySprite will always be processed, no matter the Routine SST or anything. But, evident by the conditional branch to DeleteObject, you can see how this would be a problem. Since both Routine 0/2 end with the same code, we can afford to move DisplaySprite to there, allowing DeleteObject to bypass it if needed. Just change...

    Code (ASM):
    1.         jsr Poi_Index(pc,d1.w)
    2.         bra.w   DisplaySprite
    to...

    Code (ASM):
    1.         jmp Poi_Index(pc,d1.w)
    and replace the 'rts' with the 'bra.w DisplaySprite'.

    _incObj/31 Chained Stompers.asm
    This is a combination of Bug 1 and Bug 2.

    _incObj/4E Wall of Lava.asm
    Like Bug 1. Find the second @rangechk, and move the 'bsr.w DisplaySprite' to above the 'rts' after '@moving'.
     
  3. flamewing

    flamewing

    Emerald Hunter Tech Member
    1,138
    0
    16
    France
    Sonic Classic Heroes; Sonic 2 Special Stage Editor; Sonic 3&K Heroes (on hold)
    Yeah, that caused me a major headache when I ported the S3&K BuildSprites to SCH. I suppose I ought to have written something like what you wrote back then, it would have saved a lot of headaches to other folks. Anyways, thanks for doing it.

    Technically, the bug is back in S3&K, and it is even worse: unlike S2, S3&K's BuildSprites does not check to see if an object slot is loaded, so if an object is deleted after queuing itself for drawing, it triggers the bug (another object does not need to be loaded on the same slot). This bug is the reason why the first longword in S3&K ROM is zero, instead of the initial stack frame — a hackish workaround: the mappings on the deleted object will point to this first longword, the mappings frame will be zero, so it will read an offset of 0 from position 0, then read 0 sprite pieces from position 0 and draw nothing, neutralizing the bug. I remember there were some objects in S3&K that might be affected by this issue (delete after queue), but it was a while ago (so I don't recall which ones were these) and I didn't test them either.
     
  4. ICEknight

    ICEknight

    Researcher Researcher
    The ground blocks at the Death Egg boss fight, perhaps? That's the first buggy S3K object that comes to mind...
     
  5. flamewing

    flamewing

    Emerald Hunter Tech Member
    1,138
    0
    16
    France
    Sonic Classic Heroes; Sonic 2 Special Stage Editor; Sonic 3&K Heroes (on hold)
    There are loads of buggy objects in S3K; but this bug is actually invisible in plain S3K because of the mentioned workaround — so in order to really see anything, the first longword of ROM would have to be changed to something other than 0. Followed by going through the game and trying to generate the failure conditions.
     
  6. redhotsonic

    redhotsonic

    Also known as RHS Tech Member
    1,584
    0
    0
    United Kingdom
    YouTuber

    I knew it! I bloody well knew it. When I'm back home tonight, I might see if this fix solves my issues. If it does, you're a legend!



    This is what I did to solve my MZ Boss, hasn't crashed since. I might just leave it in place in case any other objects has this bug. This S3K fix is also applied to my S2R hack as a fair few objects caused this crash, and it definitely works fine with this fix.


    EDIT: Spellings.
     
  7. flamewing

    flamewing

    Emerald Hunter Tech Member
    1,138
    0
    16
    France
    Sonic Classic Heroes; Sonic 2 Special Stage Editor; Sonic 3&K Heroes (on hold)
    I unfortunately could not use this fix in my hack(s) because of my custom linked-list object manager; I use the first word to debug linked list issues with a "trap #0" instruction (which, to be fair, hasn't been needed in quite some time...). So I had to really fix the issue in every S1 and S2 object that triggered the bug. On the other hand, I am working on an improvement to my custom object manager that will bring more speed and will fix the bug permanently — but which cannot be backported efficiently to other object managers, as it relies on the linked list.
     
  8. redhotsonic

    redhotsonic

    Also known as RHS Tech Member
    1,584
    0
    0
    United Kingdom
    YouTuber
    It was you who showed me that S3K did that trick in the first place. It did help me a lot, rather than me go bug hunting and searching for what objects caused the glitch and etc. It saved me a lot of time. I thank you for that. Shame you can't do the same thing, but that's what you get for improving other code; you break others =P
     
  9. Clownacy

    Clownacy

    Tech Members Tech Member
    775
    0
    16
    So, as Bakayote pointed out, the ending's got a small bug: you can enter debug mode in it if you hold A during the transition from Final Zone... without entering the debug mode cheat.

    The cause is simply a missing check for the debug mode cheat. Here's the code you usually see:

    Code (ASM):
    1.         tst.b   (f_debugcheat).w ; has debug cheat been entered?
    2.         beq.s   Level_ChkWater  ; if not, branch
    3.         btst    #bitA,(v_jpadhold1).w ; is A button held?
    4.         beq.s   Level_ChkWater  ; if not, branch
    5.         move.b  #1,(f_debugmode).w ; enable debug mode
    6.  
    7. Level_ChkWater:
    This is what the ending does:

    Code (ASM):
    1.         btst    #bitA,(v_jpadhold1).w ; is button A pressed?
    2.         beq.s   End_LoadSonic   ; if not, branch
    3.         move.b  #1,(f_debugmode).w ; enable debug mode
    4.  
    5. End_LoadSonic:
    So, obviously, you just gotta copy over the check, and then it'll work properly.

    Makes me wonder if debug mode didn't have a code during developement...
     
  10. DigitalDuck

    DigitalDuck

    Arriving four years late. Member
    4,770
    0
    16
    Lincs, UK
    TurBoa, S1RL
    Almost certainly not. You want to put in a code every time you test the game?
     
  11. nineko

    nineko

    I am the Holy Cat Tech Member
  12. DigitalDuck

    DigitalDuck

    Arriving four years late. Member
    4,770
    0
    16
    Lincs, UK
    TurBoa, S1RL
    How about the fact that if you go off the top of the screen when in your hurt animation, you immediately die?

    I mean, there's still no fix here/on the wiki for that one, so it must be new! :v:
     
  13. Clownacy

    Clownacy

    Tech Members Tech Member
    775
    0
    16
  14. redhotsonic

    redhotsonic

    Also known as RHS Tech Member
    1,584
    0
    0
    United Kingdom
    YouTuber

    I swear someone came up with a fix for that too, just can't find it at the moment. It's not hard to fix though, I could look into it tomorrow night (unless someone beats me to it or I forget =P).
     
  15. Qtheman

    Qtheman

    Memory Access Violation Member
    Yup, it's right here. :)
     
  16. Clownacy

    Clownacy

    Tech Members Tech Member
    775
    0
    16
    That seems to be about an already-dead Sonic getting deleted at the level boundary, not a hurt Sonic being killed by the level boundary.

    EDIT: The quote train rolls on, choo choo!
     
  17. DigitalDuck

    DigitalDuck

    Arriving four years late. Member
    4,770
    0
    16
    Lincs, UK
    TurBoa, S1RL
    That's a different issue, where if Sonic dies at the top of the screen he doesn't come back down.

    I'm talking about when Sonic gets hit near the top of the screen, he dies when he hits the top.
     
  18. Natsumi

    Natsumi

    Miss Fox Tech Member
    182
    0
    16
    Long and dangerous river
    Navigating said river
    I don't know if there is a tutorial out for this or not, however the fix is simple. Just go to Sonic_HurtStop and replace the first bcs with blt.
     
  19. nineko

    nineko

    I am the Holy Cat Tech Member
    Yup, it was Mercury, back in 2010: http://forums.sonicretro.org/index.php?showtopic=8815&view=findpost&p=470077

    Sorry for the late reply, I didn't have a chance to use my computer until now.
     
  20. DigitalDuck

    DigitalDuck

    Arriving four years late. Member
    4,770
    0
    16
    Lincs, UK
    TurBoa, S1RL