don't click here

Sonic 2 Split Disassembly

Discussion in 'Engineering & Reverse Engineering' started by FraGag, Oct 6, 2008.

  1. MoDule

    MoDule

    Tech Member
    327
    24
    18
    Procrastinating from writing bug-fix guides
    There, I fixed the conflicts. I hope I didn't undo anyone's work in the process. It doesn't look like it, but there were a lot of small differences.
     
  2. To quote changelog.txt,
    Code (Text):
    1. * The major change is the addition and usage of the macros zoneOffsetTable, zoneTableEntry and
    2.   zoneTableEnd. These macros automatically sort offset tables which use the zone ID as an offset, and thus
    3.   enable easy re-arranging of zone IDs without having to worry about re-arranging dozens of offset tables as
    4.   well. Additionally, whenever a new zone is added, the zoneTableEnd macro will automatically warn you about
    5.   which offset tables you need to add more stuff to. I've tested out some swaps and everything seems to work
    6.   properly, but I'd appreciate it if people could test different combinations out, and either fix any bugs
    7.   encountered themselves or post about them for others to fix. It should be noted that useFullWaterTables should
    8.   be set to 1 inside Assembly Options when you're shifting IDs around, and that s2p2bin will complain about
    9.   overlapping allocations, but these warnings can be safely ignored.
    I know annotative additions are probably far more useful from a practical perspective, but since FraGag had already set up his awesome dynamic IDs system this seemed a logical extension. As I said above though, I've only tested out a few swaps so far and haven't done very thorough testing, so it'd be useful if others could try changing IDs around and either fix anything which breaks or report it for fixing. The most likely causes of errors would be assumptions the programmers made (e.g. there would be no zone with water with an ID below $8, for which I had to add the useFullWaterTables option, and that EHZ would always be the first zone, for which a number of if/else statements were needed), and zone references which haven't been converted to equate form yet.
     
  3. FraGag

    FraGag

    Tech Member
    Wow, that's really cool. That's basically what I had in mind, and you executed it brilliantly! Good job!
     
  4. FraGag

    FraGag

    Tech Member
    Alright, I've been really busy lately (and I think I'm not alone :) ), but I want to make another suggestion. Basically, the CC and CS condition codes have equivalents, respectively HS (typoed as HI in the official documentation) and LO. I've often met BCC and BCS instructions that had me wondering what they did (I used to mix them up, and I probably still do), and I think it would be better to replace some instances of BCC and BCS with BHS and BLO (e.g. after CMP instructions). I feel they are the natural complements to HI and LS, which don't have equivalents.
     
  5. I agree, it would make the meaning of the branches clearer.
     
  6. Double posting since I was looking at SwScrl_EHZ and had an idea - how about including assembly options for fixing small bugs which require only a few changes to correct, such as the EHZ scrolling, placing an object after dying in debug mode, Super Sonic at the end of level, etc.? I'm not really sure if that comes under the scope of this project, which is why I wanted other people's opinions before putting it in.
     
  7. I think we should fix these problems. I mean, everyone is gonna fix them after using another copy of the disassembly so I think you should.
     
  8. Tweaker

    Tweaker

    Banned
    12,387
    2
    0
    That's dumb reasoning. The point of a disassembly is to provide an accurate recompilation of the original program when built, not an "improvement." Any macros or whatever that can be used to still achieve those means whilst keeping the unedited output the same are still fair game, but by adding arbitrary fixes that you believe to "improve" upon the game—whether they do or not—you invalidate the entire point of a disassembly in the first place.

    Honestly, leave the fixes for tutorials. We're trying to disassemble a game here, not make it better; making it better is what ROM hacking is for. :)
     
  9. MoDule

    MoDule

    Tech Member
    327
    24
    18
    Procrastinating from writing bug-fix guides
    How about adding comments in the appropriate places about the bugs without actually providing any code? For instance:

    Code (ASM):
    1. Sonic_CheckGoSuper:
    2.     tst.b   (Super_Sonic_flag).w    ; is Sonic already Super?
    3.     bne.s   return_1ABA4        ; if yes, branch
    4.     cmpi.b  #7,(Emerald_count).w    ; does Sonic have exactly 7 emeralds?
    5.     bne.s   return_1ABA4        ; if not, branch
    6.     cmpi.w  #50,(Ring_count).w  ; does Sonic have at least 50 rings?
    7.     bcs.s   return_1ABA4        ; if not, branch
    8.     ; BUGFIX:
    9.     ; branch to return_1ABA4, if the timer has stopped to fix the Super Sonic at the end of level bug
    That way people can look for "BUGFIX:" and do it themselves.
    Or, alternatively, add a separate text file containing a list of all known bugs and detailed descriptions for what causes them.
     
  10. Great idea :)
     
  11. Sik

    Sik

    Sik is pronounced as "seek", not as "sick". Tech Member
    6,718
    1
    0
    being an asshole =P
    I'd go for the text file option. Both it makes it cleaner, and "BUGFIX" sounds to me like it has already been fixed =/
     
  12. I'd prefer this over the BUGFIX thing personally, it would be easier if they're all in one place.
     
  13. FraGag

    FraGag

    Tech Member
    It could be on the wiki as well. I remember I placed a hint in the code to fix the 14 15 continues cheat jingle, but putting it in a separate file instead might be a good idea, especially for starters.

    Now that Hivebrain released his Sonic 1 disassembly on the SVN, we can try to make them work nicely together by sharing standards across them. There's already a (dying) discussion on naming conventions going on. If both disassemblies used identical names for the same things, it would make porting stuff back and forth much easier (hey, good programmers gotta be a tad lazy :P ).

    Also, I noticed that Hivebrain didn't keep the former label names in comments as is done in this disassembly. I think that they clutter the code a bit, yet it can be useful when following guides written for former versions of the disassembly. My suggestion is to put the corresponding label names in a separate text file, putting on a same line every "version" of a label, e.g.
    Code (Text):
    1. word_18E = Checksum
    2. sub_7E6 = Demo_Time = DemoTime
    3. ­
    I guess the order doesn't matter; I expect people will just search for a label, look at what else is on the line and don't care about the rest.
     
  14. Sonic Hachelle-Bee

    Sonic Hachelle-Bee

    Taking a Sand Shower Tech Member
    806
    200
    43
    Lyon, France
    Sonic 2 Long Version
    Hmm... No. This will force me to rewrite all bugs that are fixed :psyduck:

    I definitely better like this idea.

    Discussing about this disassembly, a thing that often annoy me is that there are more and more labels with a real name. I mean, while this is certainly more user-friendly for newer Sonic 2 hackers, there is no way to find quickly a subroutine when you only know its original hexadecimal address: one thing that was cool with the loc_XXXX naming sheme. I suggest to keep the loc_XXXX label next to the new label name when this is possible (at least as a comment). What do you think?
     
  15. FraGag

    FraGag

    Tech Member
    Just above, I was talking about keeping all revisions of a symbol in a separate file, to keep the ASM file clean. I guess you would prefer if they were sorted by the order they appear in the ROM, as that would make it easier to find them when editing the ROM directly, and it wouldn't hurt those who just look for an old label.

    Alternatively, we could keep all old labels still defined in the disassembly (I.e. not in comments), but in my opinion, that would just favor laziness and encourage people to keep using the older (and sometimes improper names). Speaking of improper names... some names were actually transfered to other locations that fit better, so it might be a reason why we should not do this.

    Also, with AS, you can easily generate a listing using the -L switch. This listing will have the PC of every line in the code. Also, AS adds a list of symbols (including __FORWxxx and __BACKxxx, which correspond to the nameless temporary symbols) with their values, a list of macros and a list of functions at the end of the listing. However, you will have to remove all the listing off directives from the source to get the full listing. Watch out, though, the listing is huge :) .
     
  16. Sonic Hachelle-Bee

    Sonic Hachelle-Bee

    Taking a Sand Shower Tech Member
    806
    200
    43
    Lyon, France
    Sonic 2 Long Version
    I see now. As you guessed, I prefer to have them sorted, but anyway, that's fine then :v:
     
  17. FraGag

    FraGag

    Tech Member
    An important fix has been committed. Revision 105 fixes the Sonic and Tails palette, which caused the title cards to show black instead of yellow if a level was edited in SonED2.

    See this thread for more details.
     
  18. Dr. Kylstein

    Dr. Kylstein

    Member
    86
    0
    6
    I have built S2 on Linux (with native versions of AS and the build tools), and have some things to report:
    <blockquote>AS 1.42 build 55 is required. The latest version, build 77, does not produce a bit-perfect binary and creates visible glitches in the title cards. The source to build 55 is here.
    </blockquote><blockquote>The "palette" macro converts its parameters to uppercase, preventing AS from finding the files. Switching to case sensitive mode (-U) causes the "zoneOffsetTableEntry"/"zoneTableEntry" macros break. Just rename the files to get it working.
    </blockquote><blockquote>`fixpointer` does not work properly, the resulting rom locks up just before the Sega logo. It is not necessary for unmodified or only slightly modified projects. (Is it needed at all?)

    </blockquote>Here is a bash equivalent of build.bat:
    <div class='codetop'>CODE</div><div class='codemain' style='height:200px;white-space:pre;overflow:auto'>#!/bin/bash

    # location of fixheader, fixpointers, s2p2bin
    TOOLSDIR=/opt/sonicretro
    # command to run AS
    AS=/usr/local/bin/asl
    # name of output ROM
    ROMNAME=s2built


    # make sure we can write to the file s2built.bin
    # also make a backup to s2built.prev.bin
    if [ -f "$ROMNAME".prev.bin ]; then
    if [ -w "$ROMNAME".prev.bin ]; then
    rm "$ROMNAME".prev.bin
    fi
    fi
    if [ -f "$ROMNAME".bin ]; then
    if [ -w "$ROMNAME".bin ]; then
    mv "$ROMNAME".bin "$ROMNAME".prev.bin
    else
    echo "Failed to build because write access to s2built.bin was denied."
    exit 1
    fi
    fi
    # delete some intermediate assembler output just in case
    if [ -f s2.p ]; then
    if [ -w s2.p ]; then
    rm s2.p
    else
    echo "Failed to build because write access to s2.p was denied."
    exit 1
    fi
    fi
    if [ -f s2.h ]; then
    if [ -w s2.h ]; then
    rm s2.h
    else
    echo "Failed to build because write access to s2.h was denied."
    exit 1
    fi
    fi

    # clear the output window
    clear

    # run the assembler
    # -xx shows the most detailed error output
    # -c outputs a shared file (s2.h)
    # -A gives us a small speedup
    USEANSI=n

    # allow the uer to choose to print error messages out by supplying the -pe parameter
    if [[ "%1" == "-pe" ]]; then
    "$AS" -xx -c -A s2.asm
    else
    "$AS" -xx -c -E -A s2.asm
    fi

    # if there were errors, a log file is produced
    if [ -f s2.log ]; then
    echo
    echo "**********************************************************************"
    echo "* *"
    echo "* There were build errors/warnings. See s2.log for more details. *"
    echo "* *"
    echo "**********************************************************************"
    echo
    exit 1
    fi

    # combine the assembler output into a rom
    if [ -f s2.p ]; then
    "$TOOLSDIR"/s2p2bin s2.p "$ROMNAME".bin s2.h
    fi

    if [ -f "$ROMNAME".bin ]; then
    # fix some pointers and things that are impossible to fix from the assembler without un-splitting their data
    #"$TOOLSDIR"/fixpointer s2.h "$ROMNAME".bin off_3A294 MapRUnc_Sonic $2D 0 4 word_728C_user Obj5F_MapUnc_7240 2 2 1
    # fix the rom header (checksum)
    "$TOOLSDIR"/fixheader "$ROMNAME".bin
    fi

    # done -- pause if we seem to have failed, then exit
    if [ -f s2.p ]; then
    echo
    else
    echo "Unknown failure."
    exit 1
    fi
    if [ -f "$ROMNAME".bin ]; then
    exit 0
    fi</div>
     
  19. Quickman

    Quickman

    be attitude for gains Tech Member
    5,595
    18
    18
    :x
    omg porjcet
    I'm adding the zero-displacement workaround macros in Hivebrain's Sonic 1 disassembly modified to use AS, and I think I might have found a bug.

    Code (Text):
    1. loc_1C4E8:
    2. &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;move.w&nbsp;&nbsp;&nbsp;&nbsp;$E(a1),(a6)
    3. &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;move.w&nbsp;&nbsp;&nbsp;&nbsp;0(a1),(a6); <<<
    4. &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;lea&nbsp;&nbsp;&nbsp;&nbsp;$10(a1),a1
    5. &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;dbf&nbsp;&nbsp;&nbsp;&nbsp;d1,loc_1C4E8
    6. &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;rts
    When I use the macro I get 3C00 0001, which is nonsense, rather than the expected 3CA9 0000. I've resorted to putting the correct assembled instruction in as a dc.l directive for now. Rather annoying since this is the only thing standing between me and a byte-perfect assembled Sonic 1 ROM.
     
  20. FraGag

    FraGag

    Tech Member
    There was indeed a bug with the macro, as it assumed the right operand would also generate an extension word, which isn't the case for (a6). The macro has been fixed in revision 129.

    + - I didn't write the original macro, so it's not my fault. :P