Omnimaga

Calculator Community => TI Calculators => ASM => Topic started by: aeTIos on April 23, 2011, 10:37:52 am

Title: What's wrong with this code?
Post by: aeTIos on April 23, 2011, 10:37:52 am
I have a question:
I have this code for moving an 'I' around on the homescreen, but it doesnt work well. what is wrong with it?
Code: [Select]
...(insert header here)
Xpos .equ AppBackUpScreen
Ypos .equ Xpos+1
ld A,0
ld (Xpos),A
ld (Ypos),A
Loop:
B_CALL _GetCSC
cp 1
Call Z,Yinc
cp 2
Call Z,Xdec
cp 3
Call Z,Xinc
cp 4
Call Z,Ydec
cp 15
jp Z,Quit
ld A,(Xpos)
ld  (CurCol),A
ld A,(Ypos)
ld (CurRow),A
ld HL,Itx
B_CALL _PutS
jp Loop
Xdec:
ld A,(Xpos)
cp 0
Call NZ,Adec
ld (Xpos),A
ret
Xinc:
ld A,(Xpos)
cp 15
Call NZ,Ainc
ld (Xpos),A
ret
Ydec:
ld A,(Ypos)
cp 0
Call NZ,Adec
ld (Ypos),A
ret
Yinc:
ld A,(Ypos)
cp 7
Call NZ,Ainc
ld (Ypos),A
ret
Adec:
Dec A
ret
Ainc:
Inc A
ret
Itx:                           ;"I text"
.db "I",0
Quit:
ret
I think that I am learning good atm, if you see those bigger codes that I write. But I think that you can optimize a bit too, can someone post optimized code with comments (please, the comments are very important for me)

thanks,

--aeTIos
Title: Re: What's wrong with this code?
Post by: Runer112 on April 23, 2011, 12:10:29 pm
I haven't tested the code myself, but I'm guessing the problem you're experiencing is that the 'I' sometimes moves in a random direction in addition to the direction you pressed. This is happening because the cursor position adjusting routines destroy the key value in the a register, which possibly activates another key check if the value. For instance, if Ypos was 1 and you pressed the down key, Yinc would be called. This would load Ypos into a and increase it, leaving 2 in the a register. When the Yinc routine returns, because the a register still contains 2 from the routine, it will mistakenly activate Xdec as well.

The "proper" way to fix this is to turn your key checks into a series of ElseIfs, so if one of them activates, none of the others do. The easiest way to form If and ElseIf statements is with jumps instead of calls, so I'm going to restructure your program a bit. Also note that, when forming If structures with jumps in assembly, the jumps are used to skip the portions of code you don't want, not to reach the portion of code you do want. I'm also going to add a bit of spacing, tabbing, and comments to make this a little easier to read.

Code: (Fixed code, 112 bytes) [Select]
...(insert header here)

Xpos .equ AppBackUpScreen
Ypos .equ Xpos+1

ld A,0
ld (Xpos),A
ld (Ypos),A


Loop:
B_CALL _GetCSC
cp 1
jr nz,Yincskip ;If key=1
;Yinc: ;Then (the label is commented because it isn't actually used, it's just for clarity)
ld A,(Ypos)
cp 7
Call NZ,Ainc
ld (Ypos),A
jr EndKeys ;Else...
Yincskip:
cp 2
jr nz,Xdecskip ;...If key=2
;Xdec: ;Then
ld A,(Xpos)
cp 0
Call NZ,Adec
ld (Xpos),A
jr EndKeys ;Else...
Xdecskip:
cp 3
jr nz,Xincskip ;...If key=3
;Xinc: ;Then
ld A,(Xpos)
cp 15
Call NZ,Ainc
ld (Xpos),A
jr EndKeys ;Else...
Xincskip:
cp 4
jr nz,Ydecskip ;...If key=4
;Ydec: ;Then
ld A,(Ypos)
cp 0
Call NZ,Adec
ld (Ypos),A
jr EndKeys ;Else...
Ydecskip:
cp 15
jr nz,Quitskip ;...If key=15
;Quit: ;Then
ret
Quitskip:
EndKeys: ;End (the above Else... ...If's are really ElseIfs, so one End handles them all)
ld A,(Xpos)
ld (CurCol),A
ld A,(Ypos)
ld (CurRow),A
ld HL,Itx
B_CALL _PutS
jp Loop


Adec:
dec A
ret
Ainc:
inc A
ret
Itx:                           ;"I" text
.db "I",0
Quit:
ret


I hope that restructuring makes sense. If not, feel free to ask me about it more. But that code was just to correct the key problem that you were experiencing. Time to tear this code apart and make the most optimized version I can. ;) I commented nearly every line with pseudocode that should hopefully make it easier to understand. The pseudocode is sort of a mix of TI-BASIC, Axe, and C.

Code: (Super optimized code, 52 bytes) [Select]
...(insert header here)

ld de,0 ;We will designate e=curRow, d=curCol


Loop: ;While true
B_CALL(_GetCSC) ;  getKey->key
cp 15 ;  
ret z ;  ReturnIf key=15
dec a ;  key--
jr nz,Yincskip ;  If key=0 (if original getKey=1)
;Yinc ;  Then
inc e ;    curRow++
bit 3,e ;    
jr z,EndKeys ;    If curRow=8
dec e ;    7->curRow
Yincskip: ;  End
dec a ;  key--
jr nz,Xdecskip ;  If key=0 (if original getKey=2)
;Xdec: ;  Then
dec d ;    curCol--
jp p,EndKeys ;    If curCol=-1
inc d ;    0->curCol
Xdecskip: ;  End
dec a ;  key--
jr nz,Xincskip ;  If key=3 (if original getKey=3)
;Xinc: ;  Then
inc d ;    curCol++
bit 4,d ;    
jr z,EndKeys ;    If curCol=16
dec d ;    15->curCol
Xincskip: ;  End
dec a ;  key--
jr nz,Ydecskip ;  If key=4 (if original getKey=1)
;Ydec: ;  Then
add a,e ;    
jr z,EndKeys ;    If curRow!=0
dec e ;    curRow--
Ydecskip: ;  End
EndKeys: ;  
ld (curRow),de ;  e->(curRow) ;d->(curCol)
ld a,'I' ;  
B_CALL(_PutMap) ;  Disp 'I'
jr Loop ;End



EDIT: Just came up with an even crazier, even more optimized version:

Code: (Crazily optimized code, 50 bytes) [Select]
...(insert header here)

ld de,0 ;e=curRow, d=curCol

Loop:
B_CALL(_GetCSC)
cp 15
ret z
dec a
cp 4
jr nc,EndKeys
dec a
sra a
jr nz,UpDown
;LeftRight:
ld c,d
adc a,a
add a,a
dec a
add a,d
ld d,a
and %00010000
jr z,EndKeys
ld d,c
UpDown:
ld c,e
neg
add a,e
ld e,a
and %11111000
jr z,EndKeys
ld e,c
EndKeys:
ld (curRow),de
ld a,'I'
B_CALL(_PutMap)
jr Loop
Title: Re: What's wrong with this code?
Post by: jnesselr on April 23, 2011, 12:13:26 pm
Since you are doing calls, try to xor a before returning to that cp # won't be based on x or y position.  Other than that, I'm not sure, as I would have to look at your code more.  I mean that can be heavily optomized by jumping over inc a statements instead of calling them if nz.  So if z, jump over them.
Title: Re: What's wrong with this code?
Post by: thepenguin77 on April 23, 2011, 07:13:00 pm
Edit2:
    Nvm, I lose. I didn't realize that we want to stop when we hit the walls, not loop. So just use this post as a learning exercise.

You're on runer. If we assume that the only requirements are that you are able to move the letter I around the screen, and quit, then this wins. I got it done in 40 bytes.

So "features" of this one: it shows a trail of 'I's, any key besides the arrows quits, 'I' starts at a random location (how fun!)

Code: (40 bytes) [Select]
;de = xy


loop:
bcall(_getCSC) ;a = key
dec a ;down is 1, we just keep decreasing
jr nz, notDown ;a because the keys are really nicely
;numbered for this
inc e ;e is y
notDown:
dec a ;left is 2
jr nz, notLeft
dec d ;d is x
notLeft:
dec a ;right is 3
jr nz, notRight
inc d ;d is x
notRight:
dec a ;up is 4
jr nz, notUp
dec e ;e is y
notUp:
dec a ;only things we are interested in
;0 = null, 1, 2, 3, 4
;all of these will be negative by this point
ret p ;return if it's positive

ld a, e
and 7 ;loop y at 7
ld e, a
ld a, d
and 15 ;loop x at 15
ld d, a


ld (curRow), de ;set the xy
ld a, 'I' ;set the char
bcall(_putMap) ;display it
jr loop

Too bad ld (xxxx), de is 4 bytes. I didn't know that until just now.

Edit:
    Lol, runer, mine is so much more readable than yours. I'm still not exactly sure how it works.