Your program uses many BIOS functions that require you to set the display page beforehand in the BH register. You specified it correctly in the SetCursor routine, but not in the several WriteCharacterAndAttribute calls.
mov dl, 0 ; Cursor position column
mov dh, 0 ; Cursor position row
To save space we usually write this pair of instruction as:
xor dx, dx ;Set CursorColumn=0 CursorRow=0
; Draw a really simple UI
; Draws a blue background
mov ah, 09h
mov al, 32
mov bl, 10h
mov cx, 1000h
int 10h
Here you need to verify your understanding of the count in CX. It specifies the number of characters that you want to draw. The hexadecimal value 1000h equals 4096 in decimal, but the usual 80x25 textscreen only has 2000 characters to play with! In your program it seems to work OK only because the video memory is much larger than that single display page you're looking at. Nonetheless you should correct it to avoid developing the bad habit of buffer
overflows.
; Draw a really simple UI
; Draws a blue background
mov ax, 0920h ;AH=09h Function, AL=" " Character
mov bx, 001Fh ;BH=00h Display page, BL=10h BrightWhiteOnBlue
mov cx, 2000 ;Character count
int 10h
As you can see I combined a pair of settings in a single instruction. This saves space, can be a bit faster, and the comments make it absolutely clear what is going on.
The same applies to drawing the gray bar, although it's not clear:
- why you wrote
160d with the d suffix to specify decimal, as decimal is the default in the absence of any prefix/suffix.
- what you expect to obtain with a counter of 160. It should give you a 2-rows gray bar where you might have expected a 1-row gray bar.
When it comes to the compairing section of the program you could definitely tidy up a bit. It will enhance readability a lot.
- Write the comments as tail-comments and keep them aligned
- Write short(er) comments that convey all that is needed
- Replace the ASCII codes by their actual characters
- Don't use double interlineation. (This one is debatable!)
So:
cmp al, 's' ; The 's' key moves the cursor down
je Down
cmp al, 'w' ; The 'w' key moves the cursor up
je Up
cmp al, 'a' ; The 'a' key moves the cursor left
je Left
cmp al, 'd' ; The 'd' key moves the cursor right
je Right
cmp al, ' ' ; The ' ' key makes the background of the cursor magenta
je SpaceBar
jmp main ; Jump back to main
je SpaceBar
jmp main
SpaceBar:
In a construct like this most programmers will try to save some bytes by using the opposite conditional jump and else fall through:
jne main
SpaceBar:
In this simple program it is what I would do. If the polling section were much longer I would keep what you wrote.
Right:
add dl, 1
call SetCursor
jmp main
ret
The ret instruction below a jmp is totally useless. The CPU will never reach it. Now it just consumes a byte and is testament of (lack of) skills.
In its present form your program entirely depends on the values in DL and DH having a permanent meaning. If your program were to become longer and more complex, this would no longer be a desirable approach. Then variables would be in order. An example on how to use these:
CursorCol db 0
CursorRow db 0
...
add CursorCol, 1
...
SetCursor:
mov dl, CursorCol
mov dh, CursorRow
mov bh, 0
mov ah, 02h
int 10h
ret
As a final note on the indentations that you used. I've nothing against it as long as it is used in a consistent manner. This seems not to be the case! Sometimes the editor on StackExchange/CodeReview is to blame though.
