Code Proof Reading and Optimization Thread

[Faceless Fox]

Do you have some code you need checked before using it? Place it here and if someone has the spare time they can help point out potential assembly errors, logic errors, typos, infinite loops, and the such! Remember to give context to what your code does, and if you're proof reading someone else's works it would be super helpful to explain why something may not work!
This is not a thread for support when you run into errors though, this is proof reading before you put it into your ROM hack/software.
Why could this be helpful? When writing a substantial amount of code there's a reasonable chance you can slip and forget a # somewhere or use the wrong data size and it's better to catch that before everything begins crashing and acting strange.

 

[Soldaten]

As an aside

please use the code tags when writing your code.

Thanks!

 

[MarkeyJester]

I like the idea of this thread! Perhaps it might be an idea to extend the idea to optimisation too, for example; if a code which is ran in a loop is too slow, "anyone have an idea of how to speed it up?", that sorta thing.

 

[Soldaten]

I'll add that to the thread title. Will make a good all rounder!

 

[Faceless Fox]

I'm going to kick off this thread with a big one, I attached it as a file instead of a text wall because it is nearly 300 lines of code. I rewrote the title screen mode for Flight of the Tornado from scratch to incorporate a far more flexible and modular menu system. Since it is 300 lines of code I wrote from scratch I'm hoping to have it peer reviewed because my ability to think certainly fluctuated throughout the process (That's smart talk for I wrote it at midnight and was running only on coffee)
Thanks for any and all suggestions and fixes!

 

[Inferno]

Also posting here just because I'm curious: this code is a mess and I wanna see if I can clean it up in any way that doesn't break it.
(NOTE: USES S3 OBJECT MANAGER!)
It won't be a separate file though.

Spoiler: BZ Scales Code

Obj_BZScales:
        moveq   #0,d0
        move.b  obRoutine(a0),d0
        move.w  BZScales_Index(pc,d0.w),d1
        jsr     BZScales_Index(pc,d1.w)
        move.w    scale_origX(a0),d0
        andi.w    #$FF80,d0
        move.w    (v_screenposx).w,d1
        subi.w    #$80,d1
        andi.w    #$FF80,d1
        sub.w    d1,d0
        cmpi.w    #$280,d0
        bls.w    Jmp_DisplaySprite
        move.w    obRespawnNo(a0),d0    ; get address in respawn table
        beq.w    Jmp_DeleteObject        ; if it's zero, don't remember object
        movea.w    d0,a2    ; load address into a2
        bclr    #7,(a2)    ; clear respawn table entry, so object can be loaded again
    
Jmp_DeleteObject:   
        jmp    DeleteObject    ; and delete object   
        
Jmp_DisplaySprite:
        jmp    DisplaySprite
        
; ===========================================================================
BZScales_Index:       
        dc.w Scales_Init-BZScales_Index ; 0
        dc.w Scales_Main-BZScales_Index ; 2
        dc.w Scales_Scale1-BZScales_Index ; 4
        dc.w Scales_ScaleMain-BZScales_Index ; 6
        dc.w Scales_Scale2-BZScales_Index ; 8
        dc.w Scales_ScaleMain2-BZScales_Index ; $A
        dc.w Scales_Weigh-BZScales_Index ; $C
        dc.w Scales_WeighMain-BZScales_Index ; $E
        dc.w Scales_WeighFall-BZScales_Index ; $10

scale_speed:    equ $2A   
scale_origX:    equ $30        ; original x-axis position
scale_origY:    equ $34        ; original y-axis position   
scale_address:    equ $38   
scale_parent:    equ $3C       
; ===========================================================================

Scales_Init:
        addq.b    #2,obRoutine(a0)
        move.l    #Map_Scales,obMap(a0)
        move.w    #$221,obGfx(a0)
        ori.b    #4,obRender(a0)
        move.b    #4,obPriority(a0)
        move.b    #$30,obActWid(a0)
        move.w    obX(a0),see_origX(a0)
        
        jsr    FindNextFreeObj
        bne.s    @nextscale
        move.b    #id_Scales,0(a1) ; load spikeball object
        move.b    #4,obRoutine(a1) ; use See_Spikeball routine
        move.w    obX(a0),d0
        subi.w    #24,d0
        move.w  d0,obX(a1)
        move.w    obY(a0),obY(a1)
        move.b  #1,obFrame(a1)
        move.b    obStatus(a0),obStatus(a1)
        move.l    a0,scale_parent(a1)
        move.l a1,scale_address(a0)
        
    @nextscale:   
        jsr    FindNextFreeObj
        bne.s    @weight
        move.b    #id_Scales,0(a1) ; load spikeball object
        move.b    #8,obRoutine(a1) ; use See_Spikeball routine
        move.w    obX(a0),d0
        addi.w    #24,d0
        move.w  d0,obX(a1)
        move.w    obY(a0),d0
        addi.w    #16,d0
        move.w  d0,obY(a1)
        move.b  #1,obFrame(a1)
        move.b    obStatus(a0),obStatus(a1)
        move.l    a0,scale_parent(a1)
        move.l    scale_address(a0),scale_address(a1)
        move.l a1,scale_address(a0)
        
    @weight:
        jsr    FindNextFreeObj
        bne.s    Scales_Main
        move.b    #id_Scales,0(a1) ; load spikeball object
        move.b    #$C,obRoutine(a1) ; use See_Spikeball routine
        move.w    obX(a0),d0
        addi.w    #24,d0
        move.w  d0,obX(a1)
        move.w    obY(a0),obY(a1)
        move.b  #2,obFrame(a1)
        move.b    obStatus(a0),obStatus(a1)
        move.l    a0,scale_parent(a1)
        move.l    scale_address(a0),scale_address(a1)
        
Scales_Main:
        rts
        
Scales_Scale1:
        addq.b    #2,obRoutine(a0)
        movea.l    scale_parent(a0),a1
        move.l    #Map_Scales,obMap(a0)
        move.w    #$221,obGfx(a0)
        ori.b    #4,obRender(a0)
        move.b    #4,obPriority(a0)
        move.b    #$10,obActWid(a0)
        move.w    obX(a0),scale_origX(a0)
        move.w    obY(a0),scale_origY(a0)
        
    Scales_ScaleMain:
        btst    #3,obStatus(a0)
        bne.s   @skip
        move.w    (v_player+obVelY).w,scale_speed(a0)
        
    @skip:
        move.w  #24,d1 ; '#'
        move.w  #8,d2
        move.w  #8,d3
        move.w  obX(a0),d4
        jsr   SolidObject
        
        move.w  #0,d0
        
        tst.b   $26(a0)
        bne.s   @returnpre
        
        btst    #3,obStatus(a0)
        beq.s   @returnup
        move.w  scale_origY(a0),d0
        addi.w  #16,d0
        cmp.w  obY(a0),d0
        blo.s      @returnpre
        addi.w  #4,obY(a0)
        move.w  #4,obVelY(a0)
        bra.s   @return
    
    @returnup:
        move.w  scale_origY(a0),d0
        cmp.w    obY(a0),d0
        bgt.s    @returnpre
        
        subi.w  #2,obY(a0)
        move.w  #-$2,obVelY(a0)
        bra.s   @return
        
    @returnpre:
        move.w  #0,obVelY(a0)
        
    @return:
        rts
        
Scales_Scale2:
        addq.b    #2,obRoutine(a0)
        movea.l    scale_parent(a0),a1
        move.l    #Map_Scales,obMap(a0)
        move.w    #$221,obGfx(a0)
        ori.b    #4,obRender(a0)
        move.b    #4,obPriority(a0)
        move.b    #$10,obActWid(a0)
        move.w    obX(a0),scale_origX(a0)
        move.w    obY(a0),scale_origY(a0)
        
    Scales_ScaleMain2:
        move.w  #24,d1 ; '#'
        move.w  #8,d2
        move.w  #8,d3
        move.w  obX(a0),d4
        jsr   SolidObject
        
        move.w  #0,d0
        
        move.l a1,-(sp)
        movea.l    scale_address(a0),a1
        
        move.w  scale_speed(a1),scale_speed(a0)
        
        tst.b    $27(a0)
        beq.w  @test2
        
        move.w  scale_origY(a0),d0
        cmp.w  obY(a0),d0
        ble.s  @unset
        
        move.b  #1,$26(a1)
        addi.w    #4,obY(a0)
        move.w  #4,obVelY(a0)
        subi.w    #4,obY(a1)
        clr.w   obVelY(a1)
        bra.s   @test2
        
    @unset:
;        illegal
        move.l a2,-(sp)
        
        lea    (v_player).w,a2
        
        btst    #3,obStatus(a1)
        beq.s   @restore
        
        move.w    #-$800,obVelY(a2)
        cmpi.w  #$800,scale_speed(a1)
        ble.s   @skip
        move.w    #-$A00,obVelY(a2)
        cmpi.w  #$A00,scale_speed(a1)
        ble.s   @skip
        move.w    #-$C00,obVelY(a2)
    @skip:
        bset    #1,obStatus(a2)
        bclr    #3,obStatus(a2)
        clr.b    jumping(a2)
        clr.b    spindash_flag(a2)
        move.b    #id_Spring,obAnim(a2)
        sfx    sfx_Spring,0,0,0

@restore:       
        move.l (sp)+,a2
        
        clr.b  $26(a1)
        clr.b    $27(a0)
        move.l (sp)+,a1
        bra.s   @return
;        subi.w    #1,(v_player+obY).w
;        illegal
        
    @test2:
        move.w  obVelY(a1),d0
        neg.w   d0
        move.w  d0,obVelY(a0)
        add.w   d0,obY(a0)
        
        move.l (sp)+,a1
        
        move.w  scale_origY(a0),d0
        subi.w  #16,d0
        cmp.w   obY(a0),d0
        blt.s   @returnpre
        
        move.b  #1,$26(a0)
        bra.s   @return
        
    @returnpre:
        clr.b  $26(a0)
    
    @return:
        rts
        
Scales_Weigh:
        addq.b    #2,obRoutine(a0)
        movea.l    scale_parent(a0),a1
        move.l    #Map_Scales,obMap(a0)
        move.w    #$221,obGfx(a0)
        ori.b    #4,obRender(a0)
        move.b    #4,obPriority(a0)
        move.b    #$10,obActWid(a0)
        move.w    obX(a0),scale_origX(a0)
        move.w    obY(a0),scale_origY(a0)
        
    Scales_WeighMain:
        move.w  #16,d1 ; '#'
        move.w  #12,d2
        move.w  #12,d3
        move.w  obX(a0),d4
        jsr   SolidObject
        
        move.w  #0,d0
        
        move.l a1,-(sp)
        movea.l    scale_address(a0),a1
        tst.b   $27(a1)
        beq.s   @noconcern
        
        move.w  obY(a1),d0
        subi.w  #16,d0
        move.w d0,obY(a0)
        bra.s  @return
        
    @noconcern:
        move.w  obVelY(a1),d0
        add.w   d0,obY(a0)
        
        move.w  scale_origY(a1),d0
        subi.w  #16,d0
        cmp.w  obY(a0),d0
        bgt.s  @checks
        clr.b  $26(a0)
        
    @checks:
        tst.b   $26(a1)
        beq.s   @return
        
        tst.b   $26(a0)
        bne.s      @return

        
        addq.b  #2,obRoutine(a0)
        sfx    $C2,0,0,0
        move.w  #-$600,obVelY(a0)
        cmpi.w  #$800,scale_speed(a1)
        ble.s   @skip
        move.w    #-$700,obVelY(a0)
        cmpi.w  #$A00,scale_speed(a1)
        ble.s   @skip
        move.w    #-$800,obVelY(a0)
    @skip:
        move.l (sp)+,a1
        jsr    ObjectFall
        bra.s  Scales_WeighFall
        
;    @begindrag:
;        move.w  scale_origY(a1),d0
;        cmp.w   obY(a1),d0
;        bge.s   @return   

;        move.b  #1,$27(a1)
        
    @return:
        move.l (sp)+,a1
        rts
        
Scales_WeighFall:
        move.w  #16,d1 ; '#'
        move.w  #12,d2
        move.w  #12,d3
        move.w  obX(a0),d4
        jsr   SolidObject
        
        move.l a1,-(sp)
        movea.l    scale_address(a0),a1
        move.w  obY(a1),d0
        subi.w  #16,d0
        cmp.w   obY(a0),d0
        blt.s   @donepre
        jsr    ObjectFall
        bra.s   @done
        
    @donepre:
        subq.b  #2,obRoutine(a0)
        move.w  obY(a1),d0
        subi.w  #16,d0
        move.w d0,obY(a0)
        move.b #1,$26(a0)
        move.b #1,$27(a1)
        
    @done:
        move.l (sp)+,a1
        rts

 

[DeltaWooloo]

I'll throw a bit of code I need a bit of cleaning:

Spoiler: Shield deflect

Touch_ChkHurt:
        move.b    ($FFFFFE2C).w,d0   
        and.b    #$E,d0        
        beq.s    Touch_ChkHurt_NoShield
        btst    #0,$2C(a1)      
        bne.s    Touch_ChkHurt_Bounce   
        and.b    $2C(a1),d0       
        beq.s    Touch_ChkHurt_NoShield   
        bra.s    loc_1AFE6               

Touch_ChkHurt_Bounce:

I think it's a mess and I really think this should also need a clean-up but I don't know where to get started. Is it worth re-writing the code?

 

[lavagaming1]

hi this is my obj manager i kinda feel insecure about

ObManagerRamLayoutSize = $A

Load_Sprites:
         tst.b  Obj_placement_routine.w
         bne.w  Load_SpriteCustom

; loc_17AB8
Load_Sprites_Init:
        move.l    #Obj_Index,d0
        tst.b   (s3k_level_mode).w
        beq.s   NotS3klisting
        cmpi.b  #green_hill_zone,(Current_Zone).w   ; is this ghz slot ?
        beq.s   NotS3klisting ; yes go to normal thing

NormalS3kList:
        move.l    #Sprite_ListingK,d0
        bra.w   loc_1B6CA
 NotS3klisting:
    move.l    #Obj_Index,d0

loc_1B6CA:
        move.l    d0,(Object_index_addr).w
    addq.b    #4,(Obj_placement_routine).w

    lea    (Object_Respawn_Table).w,a0
    move.w    #(Object_Respawn_Table_End-Object_Respawn_Table)/4,d1 ; set to clear $2FE bytes

loc_1B6E2:
    clr.l    (a0)+
    dbf    d1,loc_1B6E2
loc_1B6E8:
    move.w    (Current_ZoneAndAct).w,d0 ; If level == $0F01 (ARZ 2)...
    ror.b    #1,d0
    lsr.w    #5,d0
        tst.b   (s3k_level_mode).w
        beq.s   +
    lea    (Off_S3ObjLayout).l,a0
        bra.s  ++
+
    lea    (Off_Objects).l,a0    ; Next, we load the first pointer in the object layout list pointer index,  Off_S3ObjLayout

+
        movea.l    (a0,d0.w),a0

        moveq    #0,d0 ; clear zone calculations
        move.w  (a0)+,d0 ; get our header thanks object file +2
        move.w  d0,ObManagerObjsAmount.w
        lea     (ObjectLayoutBuff).l,a4   ; this is the Ram buffer

.loopCopyIntoRam:
        move.l  (a0)+,(a4) ; copy first object 4 bytes
        move.l  (a0)+,4(a4)   ; copy extra stuff subtype and flags and ids
        clr.w   $8(a4)
        lea     ObManagerRamLayoutSize(a4),a4
        dbf     d0,.loopCopyIntoRam ; loop
        rts

Load_SpriteCustom:
          move.w   ObManagerObjsAmount.w,d0
          lea     (ObjectLayoutBuff).l,a0

          move.w   #$3800,d6 ; load object layout RAM start
ObManagerCycleLayout:
          move.w  d6,d4
          addi.w   #ObManagerRamLayoutSize,d6 ; incress LayoutRam addr by 8
          move.w  (a0),d1 ; load x pos
          move.w  2(a0),d2
          sub.w   (Player_1+x_pos).w,d1
          bpl.s   ObManagerPlayerlvlDirection
          neg.w   d1
 ObManagerPlayerlvlDirection:
          cmpi.w  #$128,d1  ; is object away from sonic 128 pixels from screen ?
          bhs.s   ObManagerCycleObjs;.ObjSetOffScreen ; if not check other objs

          tst.w   4(a0) ; is object set to not respawn ?
          bmi.s   ObManagerCycleObjs  ; if so then cycle other objects ignore spawning it
          tst.b   $9(a0) ; has the obj been loaded ?
          bne.s   ObManagerCycleObjs ; if so ignore spawning it
 ObManagerSpawnObj:
          lea    (Dynamic_Object_RAM).w,a1
          jsr     Create_New_Sprite3.loop
          move.w  $0(a0),x_pos(a1)
          move.w  $2(a0),d1 ;2,3
          move.w   d1,d5
          andi.w   #$FFF,d1
          rol.w       #3,d5
          andi.w   #3,d5
      move.b   d5,render_flags(a1)
      move.b   d5,status(a1)
          move.w   d1,y_pos(a1)

          movea.l  (Object_index_addr).w,a4 ; faster than the other method by 2 cycles ig
          add.w   4(a0),a4  ;4,5
          move.l   (a4)+,(a1)


          move.w  6(a0),subtype(a1)
          move.b  #1,$9(a0)
          move.w  d4,respawn_addr(a1); get our location in layout RAM
 ObManagerCycleObjs:
          lea     ObManagerRamLayoutSize(a0),a0
          dbf     d0,ObManagerCycleLayout
          rts

how fast is this code and how would adding a y pos check look like and will doing that make this faster than s3k obj manager ?