Author Topic: What's wrong with this code?  (Read 1961 times)

0 Members and 1 Guest are viewing this topic.

Offline aeTIos

  • Nonbinary computing specialist
  • LV12 Extreme Poster (Next: 5000)
  • ************
  • Posts: 3914
  • Rating: +184/-32
    • View Profile
    • wank.party
What's wrong with this code?
« 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
I'm not a nerd but I pretend:

Offline Runer112

  • Moderator
  • LV11 Super Veteran (Next: 3000)
  • ***********
  • Posts: 2289
  • Rating: +639/-31
    • View Profile
Re: What's wrong with this code?
« Reply #1 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
« Last Edit: April 23, 2011, 02:08:01 pm by Runer112 »

Offline jnesselr

  • King Graphmastur
  • LV11 Super Veteran (Next: 3000)
  • ***********
  • Posts: 2270
  • Rating: +81/-20
  • TAO == epic
    • View Profile
Re: What's wrong with this code?
« Reply #2 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.

Offline thepenguin77

  • z80 Assembly Master
  • LV10 31337 u53r (Next: 2000)
  • **********
  • Posts: 1591
  • Rating: +823/-5
  • The game in my avatar is bit.ly/p0zPWu
    • View Profile
Re: What's wrong with this code?
« Reply #3 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.
« Last Edit: April 23, 2011, 07:16:38 pm by thepenguin77 »
zStart v1.3.013 9-20-2013 
All of my utilities
TI-Connect Help
You can build a statue out of either 1'x1' blocks or 12'x12' blocks. The 1'x1' blocks will take a lot longer, but the final product is worth it.
       -Runer112