(quoting from the Sonic 1 disassembly thread to avoid taking that off-topic) Hmm do you mind elaborating on this a bit more? It's something we can look into fixing.
Double posting but whatever. What do you guys think of creating a downloadable version of the current revision of this disassembly, similar to what Hivebrain's done for the Sonic 1 disasm? Are there any particular changes you'd like to make before any such step is taken, or is it fine in its current state?
I'm all for it. I've still got a few things I haven't committed yet, though I really only feel I need to ask about one: What's everyone's opinion on giving all the unknown RAM addresses generic unk_*(address) names? It would make rearranging the RAM layout a lot easier. If everyone's okay with that I'll go ahead and commit my changes (though I'm still missing a few addresses used during the special stages which I can't make heads nor tails of).
Well, let's say for example this line during the Vertical Blanking Interval (Sonic 1): Code (ASM): writeVRAM v_spritetablebuffer,$280,vram_sprites ...Of which is an equivilent to the original assembly mnemonics: Code (ASM): lea ($C00004).l,a5 move.l #$94019340,(a5) move.l #$96FC9500,(a5) move.w #$977F,(a5) move.w #$7800,(a5) move.w #$0083,($FFFFF640).w move.w ($FFFFF640).w,(a5) This is responsible for setting the VDP, to DMA transfer sprite data from the sprite table buffer (00FFF800) to the VDP's sprite table V-Ram location (incidentally F800), allowing the MC68 to do other tasks. The issue I find here is that the first one (Running under a Macros) although it's easier to edit by all means (why hell it is rather useful in all honesty), most people would edit/use it there entire lives and not understand WHY it works or even HOW, their knowledge is therefore limited and their understanding of the VDP and it's registers, may never occur. The possibilities are almost endless with the VDP, and understanding how and why it works (Given the original code as help) would help one benefit from a learning point of view. That was basically what I meant, not to say that I'm "complaining" per se, but I felt that it should at least be noted for future reference.
I'm okay with it (and I'm kinda used to it from IDA anyway). We should probably add a readme.txt of sorts explaining some of the major changes that have been made (such as the dynamic IDs system and the new way of defining RAM equates) so that newcomers don't get overwhelmed. I see your point, but the benefits far outweigh the drawbacks in this case IMO, and if someone's keen on learning how to program the VDP manually they can always take a look at the macro source or a document such as genvdp.txt.
Yes please. There should be no literal absolute RAM address left in the source, so that absolutely nothing breaks when moving stuff around. Ideally, some offsets on registers that point to RAM should also be labelled somehow (possibly using subtraction with 2 labels), but it might be a bit more difficult to do if it's not obvious what the register can point to.
Well, I guess I can go ahead and commit what I've got. I'm afraid there might still be some RAM addresses missing (besides the special stage ones, that is). I don't suppose there's a smart way to find RAM addresses that don't have the $FFFF prefix? I mean stuff like this: Code (ASM): ; word_917A: OptionScreen_Choices: dc.l (3-1)<<24|(Player_option&$FFFFFF) dc.l (2-1)<<24|(Two_player_items&$FFFFFF) dc.l ($80-1)<<24|(Sound_test_sound&$FFFFFF)
Can we shorten some of the equate names a bit? E.g. ehz_1 instead of emerald_hill_zone_act_1, GMID_* instead of GameModeID_*, and bit_* instead of button_* and btn_* instead of button_*_mask (copying Hive's naming scheme for this one).
Because having to type GameModeID_ContinueScreen in your own code would get kinda annoying (though admittedly GMID_* is only 6 characters shorter). emerald_hill_zone_act_1 vs. ehz_1 is a saving of 18 characters, however, and I'm sure everyone knows what ehz is.
I definitely agree for the zones/acts constants. For GameModeIDs, I'm not sure; on one hand, as ICEknight said, acronyms/abbreviations may be cryptic until you look them up. On the other hand, they always get moved to Game_Mode. Having "game mode" spelled out twice on the same line is redundant, but then shortening "GameModeID" to "GMID" can lead to conflicts if something else is shortened to "GMID". That said, then I made all those IDs, I used shorter names for the others, so we might as well just go with GMID...
A universal labelling scheme would make some matters easier for both SVN disassemblies. This would at least make a start to it.
...Perhaps we could start by using the known labels from the original source files by Sonic Team? We have a few of them, don't we?
We have label names found in the symbol tables of the S&KC exe. That said, I dunno if a universal naming scheme would extend nicely to other games. Maybe it should stay with S1-3K only, but first we need to finish up our S&K split. And even if we do that, would having to abandon descriptive labels for the somewhat constrained ones Sonic Team used be worth it? As far as abbreviations go, I don't see how abbreviating zone names would not be common sense. As far as everything else goes, you are probably better off leaving them alone.
I thought about that, but decided against it because it wouldn't really be helpful to us. Programmers don't always use labels with clear meanings to anyone but themselves, especially if they don't expect their code to be made public.
bit_* is too generic. Perhaps btnb_* (button bit) and btnm_* (button mask)? We could also drop the underscore entirely.
Sounds good to me, although I'd suggest keeping the underscore for consistency with other equates. We should also probably have btnm_ABC and btnm_dir (or btnm_UDLR, whichever one is preferred).
I've shortened the equates and was commenting the Enigma decompression routine when I ran into a little hitch: Code (ASM): move.w d5,d1 move.w d6,d7 sub.w a5,d7 ; subtract length in bits of inline copy value bhs.s $$enoughBits ; branch if a new word doesn't need to be read move.w d7,d6 addi.w #16,d6 neg.w d7 ; calculate bit deficit lsl.w d7,d1 ; and make space for that many bits move.b (a0),d5 ; get next byte rol.b d7,d5 ; and rotate the required bits into the lowest positions add.w d7,d7 and.w EniDec_Masks-2(pc,d7.w),d5 add.w d5,d1 ; combine upper bits with lower bits $$maskValue: move.w a5,d0 ; get length in bits of inline copy value add.w d0,d0 and.w EniDec_Masks-2(pc,d0.w),d1 ; mask value appropriately add.w d3,d1 ; add starting art tile move.b (a0)+,d5 lsl.w #8,d5 move.b (a0)+,d5 ; get next word rts ; =========================================================================== $$enoughBits: beq.s $$justEnough ; if the word has been exactly exhausted, branch lsr.w d7,d1 ; get inline copy value move.w a5,d0 add.w d0,d0 and.w EniDec_Masks-2(pc,d0.w),d1 ; and mask it appropriately add.w d3,d1 ; add starting art tile move.w a5,d0 bra.s EniDec_FetchByte ; =========================================================================== $$justEnough: moveq #16,d6 ; reset shift value bra.s $$maskValue ; End of function EniDec_FetchInlineValue AS complains about symbol undefined errors in the second pass even though I don't have any non-temporary labels in between the named temporary ones and so there shouldn't be any issues. Anybody know why this is happening? Using nameless temporary symbols instead works fine but I'd prefer to keep the named ones.