MultiplexorMan wrote
(LOOP)
@SCREEN
D=A
@i
M=D
@24576
D=M
@WHITE
D;JEQ
@24576
D=M
@BLACK
D;JGT
A problem here is that you are checking the keyboard buffer two different times with no assurance that it will be the same value both times. So a key could be down when you check it the first time (and thus not make the jump) and up when you check it the second time (and again not make the jump). In this case the consequences aren't too severe since it would flow into the (WHITE) section of the code and thus the behavior is that it will assume that no key is pressed in this situation. But in general the results could be very bad.
A good practice is to only check the keyboard status once during each pass of the loop and take the appropriate action based on that one test.
Use a template for a high level selection construct. For instance
if(test)
if_code
else
else_code
next_code
The three blocks of code could be located anywhere in the program (not just one after another), so perhaps
(IF_CODE)
the assembly code for the if-block
@NEXT_CODE
0; JMP
...
(ELSE_CODE)
the assembly code for the else-block
@NEXT_CODE
0; JMP
...
(NEXT_CODE)
the assembly code that follows the if() statement
Now we just have to handle the test.
assembly code to put the value of test in D
@IF_CODE
D; JNE
@ELSE_CODE
0; JMP
If the value in D is anything other than zero, this will jump to the if-block. Otherwise, if it doesn't jump, we can conclude that the value in W was zero and jump to the else-code.
If either the if-block or the else-block immediately follows the test code, then we can eliminate the unconditional jump. Since we normally think of the if-block being before the else-block, we can write it in that order.
assembly code to put the value of test in D
@ELSE_CODE
D; JEQ
(IF_CODE)
the assembly code for the if-block
@NEXT_CODE
0; JMP
(ELSE_CODE)
the assembly code for the else-block
(NEXT_CODE)
the assembly code that follows the if() statement
Notice how we used the natural flow through to eliminate all but one of the unconditional jumps.
(WHITE)
@i
A=M
M=0
D=A+1
@i
M=D
@24576
D=M
@WHITE
D;JEQ
@LOOP
0;JMP
What you have done here is create a loop within a loop, which is probably not what you intended.
Also, you don't prevent your code from writing to memory outside of the screen map; thus you end up with an array-bounds error.
What happens when when 'i' gets to be 24576 (i.e., right after you turn the last sixteen pixels white)? If no key is pressed, you loop back up to (WHITE) and then write a zero into 24576 (the keyboard buffer) and advance 'i' to 24577. I'm not sure how the simulator handles attempts to write to the buffer; I'd have to play around with it since I believe that this is undefined behavior. But if no key is pressed or if it overwrote whatever key code was there, it will loop back up to WHITE again and now attempt to write to 24577 which the simulator apparently is catching and throwing an error.
(BLACK)
@i
A=M
M=-1
D=A+1
@i
M=D
@24576
D=M
@BLACK
D;JGT
@LOOP
0;JMP
Here you have the same issue, except that if writes to the keyboard buffer are allowed you end up writing -1 to it which means it will fail the JGT test and go back up to (LOOP) at which point 'i' gets reset to the beginning of the screen buffer.
So you can see that even a tiny difference in the code can lead to very different behaviors.
Locked
