Had another look at the code, and I'd like to comment on a few things.
In the initialisation, the RAM clear has me a bit undecided...
cld ; clear BCD bit
ldx #$FF
txs ; set stack to the beginning
ldy INTIM ; for random number seed
lda #$00
.clearRAM
sta $00,x
dex
bne .clearRAM
Firstly, it's really clearing RAM *and* hardware registers. If you just
wanted to clear RAM, then you could change the bne .clearRAM to a bmi
.clearRAM (keeping in mind that X would be $7F at the end of that, instead
of 0 - but it doesn't seem to be used anyway). But clearing all of RAM (but
1) seems kind of hackish. Either clear it all, or clear RAM.
Secondly, there were some discussions on a minimal (byte-count) kernel a
long time ago, and I posted an efficient startup at that time. Here it
is... it's a bit bizarre, but very efficient :)
; CLEARS ALL VARIABLES, STACK
; INIT STACK POINTER
; ALSO CLEARS TIA REGISTERS
; DOES THIS BY "WRAPPING" THE STACK - UNUSUAL
LDX #0
TXS
PHA ; BEST WAY TO GET SP=$FF, X=0
TXA
CLEAR PHA
DEX
BNE CLEAR
; 9 BYTES TOTAL FOR CLEARING STACK, MEMORY
; STACK POINTER NOW $FF, A=X==0
I have to say, that's so elegant I have tears in my eyes :)
Somebody better check it, though - the brilliance is blinding me :P
One thing I've noticed about your code, Dennis, is your use of two-digits
for hex bytes, *ALWAYS*. This is not necessary. It's quite OK to go sta
0,x or sta $00,x or sta $0,x -- they are all equivalent. I used to go lda
#$02, too! It's a style thing, I know. But sometimes using #$00 is a bit
of overkill, when #0 is exactly the same, and much more readable (IMHO).
You init the random number from INTIM - yet you don't check if this is 0.
The RandomByte routine documents this must *not* be zero. Looking at the
routine, it should cope with 0 OK - but you should either fix the code or
the comments.
You tend to do a lot of NTSC/PAL checking, with branching sections of code
for loading NTSC or PAL conditions/colours/states. This strikes me as
inefficient. I'd probably check the switch at startup and save the relevant
bit as D0 of a variable (or D1 - see later in this post). Then you could
use that variable as an index to tables.
For example, the title screen colours is currently....
TitleScreenCalculations SUBROUTINE
lda #NTSC_SKY_BLUE ; set the NTSC colors for the title
ldx #NTSC_WOOD_BROWN
bit SWCHB ; check the right difficulty switch
bpl .setTitleScreenColors ; if set to B then use NTSC settings
lda #PAL_SKY_BLUE ; else use PAL settings
ldx #PAL_WOOD_BROWN
.setTitleScreenColors
sta COLUBK
stx COLUPF
With a variable, and tables, that would be...
TitleScreenCalculations SUBROUTINE
ldy PAL_NTSC ; the stored NTSC/PAL switch (bit D0
set or clear)
lda SKYColour,y
sta COLUBK
lda WOODColour,y
sta COLUPF
...
SKYColour .byte NTSC_SKY_BLUE, PAL_SKY_BLUE
WOODColour .byte NTSC_WOOD_BROWN, PAL_WOOD_BROWN
This code only benefits from being constant cycle-time - on average faster,
and more readable. It requries a RAM variable to save the switch state,
though. You do a fair bit of SWCHB checking - it would be nice to clean
those conditional bits of code up. This is optional, though - some people
might prefer your original.
By the way, your use of SUBROUTINE is interesting. Just to clarify things,
this is *NOT* necessary - it just gives a sort of local-scope to the labels
beginning with a dot (.) It would be nice for DASM to be able to associate
overlays with subroutine sections via the use of a name/overlay list as
parameters for the subroutine. Just an idea for me to think about :)
Here's a minor optimisation...
.doDeathAnimation
lda gameClock ; get the game clock for player
animation
lsr ; updated every other frame
and #$01 ; mask all but D0 alternate between 0
and 1
tax ; move value to x for table index
lda LowDeathAnimationTable,x ; load the climber death animation
sta playerPointer ; into the player pointers
lda HighDeathAnimationTable,x ; they will be used to load RAM with the
sta playerPointer+1 ; death animation
OK, so instead of having two tables, have one word-table, and save on the
lsr.
ie:
.doDeathAnimation
lda gameClock
and #$02
tax
lda DeathAnimationTable,x
sta playerPointer
lda DeathAnimationTable+1,x
sta playerPointer+1
...
DeathAnimationTable
.word DeathFrame1, DeathFrame2
Just below that is another example of SWCHB optimisation with a variable.
Use this instead...
lda PAL_NTSC
asl
tax
ldy DeathAnimationColorTable,x ; y holds the low byte of the color
table
lda DeathAnimationColorTable+1,x ; a holds the high byte of the color
table
jmp .setVerticalColorPointer ; set the color pointers
This alone saves 4 bytes (see your label .loadDeathColors for the location).
In fact, if you use SWCHB to pretty much index word tables (or can contrive
to), then you might be better setting D1 of the PAL_NTSC variable instead of
D0. Then you don't need to ASL, or lda.
ldx PAL_NTSC ; 0 or 2
ldy DeathAnimationColorTable,x ; etc
and the earlier code using SWCHB stuff would be as simple as merging the
tables...
ldy PAL_NTSC ; the stored NTSC/PAL switch (bit D1
set or clear)
lda Colour,y
sta COLUBK
lda Colour+1,y
sta COLUPF
...
Colour
.byte NTSC_SKY_BLUE, NTSC_WOOD_BROWN
.byte PAL_SKY_BLUE, PAL_WOOD_BROWN
Finally, your code had....
overlay ds 27 ; overlay place holder (thank you
Andrew)
You are very welcome. Seeing a comment like that makes it all worthwhile -
thank you!
Cheers
A
----------------------------------------------------------------------------------------------
Archives (includes files) at http://www.biglist.com/lists/stella/archives/
Unsub & more at http://www.biglist.com/lists/stella/
