##Analyses (in random order)
mov ah, 00h
mov al, 03h
int 10h
mov ah,04ch
mov al,00h
int 21h
We always strive to get the smallest code without loosing readability. I think next snippet does a good job (it's 2 bytes smaller and well commented):
mov ax, 0003h ;AH=00h BIOS.SetVideo, AL=3 Textmode 80x25
int 10h
mov ax, 4C00h ;AH=4Ch DOS.Terminate, AL=0 ExitCode
int 21h
mov ax, 10
mov currentx, ax
mov ax, 10
mov currenty, ax
mov ax, 2
mov currentshape, ax
mov ax, 0
mov changeshape, ax
It's perfectly possible to move the numbers in the variables without putting them in a register first. But in the case where the same number goes into more than 1 variable using especially the AX register is advantageous as it reduces code size.
mov ax, 10
mov currentx, ax
mov currenty, ax
mov currentshape, 2
mov changeshape, 0
mov ax, 1
add currentx, ax
mov ax, currentx
The 3rd instruction here tells me that the value 1 that you moved into the AX register isn't all that important after the addition, so why not just increment the currentx variable?
inc currentx
mov ax, currentx
mov al, colour
inc al
mov colour, al
Again a perfect occasion to use less instructions and just increment the variable in one go:
inc colour
pushall
mov ax, py
mov bx, 320
mul bx
add ax, px
mov di, ax
mov al, colour
mov es:[di],al
popall
The plot macro code can be a bit smaller and use a register less if you wrote:
pushall
mov ax, 320
mul py <-- No need to use BX here
add ax, px
mov bx, ax
mov al, colour
mov es:[bx], al
popall
Note that I didn't use the DI register like you did. Because the pushall and popall macros don't actually preserve it, this could easily become a death-trap!
Although using macros to push/pop registers is easy, it also brings about pushing/popping too much, like in this code the CX register that isn't used at all.
pushall
mov ax, tx
add ax, tw
mov tx2,ax
mov bx, tw
shr bx, 1
add bx, tx
mov tx3, bx
mov cx, ty
sub cx, th
mov ty3, cx
line tx, ty, tx2, ty
line tx2, ty, tx3, ty3
line tx3, ty3, tx, ty
popall
I'll use the triangle code to show many opportunities to shorten the code. mov bx, tw was moved to the top to avoid reading the tw variable twice.
push ax <-- Shorter than the pushall code
push bx
mov bx, tw
mov ax, tx
add ax, bx <-- Shorter between 2 registers
mov tx2, ax
shr bx, 1
add bx, tx
mov tx3, bx
mov ax, ty <-- Shorter using AX instead of CX
sub ax, th
mov ty3, ax <-- Shorter using AX instead of CX
line tx, ty, tx2, ty
line tx2, ty, tx3, ty3
line tx3, ty3, tx, ty
pop bx
pop ax <-- Shorter than the popall code
Similar code to the next one is often repeated in the draw macro.
mov ax, 0
add ax, currentx
mov val1, ax
mov ax, 50
add ax, currenty
mov val2, ax
mov ax, 50
mov val3, ax
mov ax, 43
mov val4, ax
Why not simple mov the variable in the register?
mov ax, currentx
mov val1, ax
Changing the order gives shorter code:
mov ax, currenty
add ax, 50
mov val2, ax
Move the number in one go:
mov val3, 50
mov val4, 43
#Conclusion
As you can see it's possible to optimize this code a lot, but I think the best advice I can offer is that all these macros really should have been written as subroutines. In this form the program is simply too long for what it does.
I tried to not repeat comments that I made on the previous (shorter) review. Obviously they still count.
I'm pretty sure that if all these modifications were applied correctly, and thus shortening the program considerably, more advanced tips (especially on program flow) could be given.
