A simple sprite demo for teaching

Discussion of hardware and software development for Super NES and Super Famicom. See the SNESdev wiki for more information.

Moderator: Moderators

Forum rules
  • For making cartridges of your Super NES games, see Reproduction.
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

I don't have a Windows+MSYS+Python installation handy to troubleshoot it myself, but this user has the same problem you're having. You might need to put Python on whatever MSYS thinks is your PATH as well.
User avatar
bigjt_2
Posts: 82
Joined: Wed Feb 10, 2010 4:00 pm
Location: Indianapolis, IN

Re: A simple sprite demo for teaching

Post by bigjt_2 »

Got er! It ended up being pretty simple. Python doesn't automatically set up an Path environment variable when it's installed on Windows, so I did that myself in the Advanced System Settings under System.

I also had an issue with directory/executable naming for no$sns. I left the executable named that when it unzipped, and unzipped everything for no$sns into a directory called no$sns months ago when I started playing around with it. The Make script didn't like the $ character, so I just renamed the directory and executable and all was well.

The ROM has compiled and is now up and running fine. Thanks for taking the time to make and share this Tepples. I've been off and on experimenting with SNES for a few months now and a nice beginner example like this will be very fun/helpful to dig into.
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

Egg is on my face. I had forgotten to include an essential file. This has been corrected.
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

So it appears my code is unreadable.
In [url=http://forums.nesdev.com/viewtopic.php?p=140215#p140215]this post[/url], koitsu wrote:I finally got around to looking at this code. Ho-ly-shit. Really? God dude, I don't even know where to begin. The code itself is fine (sort of -- very bad init routine from the look of it)
"very bad init routine"
Does it write the right values to the right registers? Is accessing them using D at $2100 a deprecated practice?
but it's nearly impossible to follow given formatting, things in files that don't make any sense (like why is the reset vector code in snesheader.s)
For the same reason that C libraries include a pile of code that runs before the jump to main() in "crt0.s".
and a ton of other things. Can you honestly read this given the formatting and almost "inline" comments without any actual structure?
What sort of "structure" should have been used?
User avatar
Drew Sebastino
Formerly Espozo
Posts: 3496
Joined: Mon Sep 15, 2014 4:35 pm
Location: Richmond, Virginia

Re: A simple sprite demo for teaching

Post by Drew Sebastino »

tepples wrote:Quote:but it's nearly impossible to follow given formatting, things in files that don't make any sense (like why is the reset vector code in snesheader.s)For the same reason that C libraries include a pile of code that runs before the jump to main() in "crt0.s"
You know the primary programming language here isn't C, right? :wink: (Hell, I don't even know anything except ASM. Everything else might as well be scribbles for all I care.)

I thing one problem is that everything is shoehorned into only a couple of files. This is alright for if you made it and know where everything is, but if other people are looking at it, you might want to have things like the init code separate. You might want to look at Walker on the SNES starterkit. Its very organized and understandable, in my opinion.
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

I agree that a map of the source code would help. Might it look like the following?

Organization of the program

Include files
  • snes.h: Register definitions and some useful macros for the S-CPU
  • global.h: S-CPU global variable and function declarations
  • spc-ca65.inc: Macro library to produce SPC700 instructions
  • spc-65c02.inc: Macro library to use 65C02 syntax with the SPC700
Source code files (65816)
  • snesheader.s: Super NES internal header and initialization code
  • main.s: Main program
  • ppuclear.s: Useful low-level subroutines for interacting with the S-PPU
  • blarggapu.s: Send a sound driver to the S-SMP
Source code files (SPC700)
  • spcheader.s: Header for the .spc file; unused in .sfc
  • spcimage.s: Sound driver
(end)

If too much code is in main.s, tell me what you think should be split and why, and I'll consider it.
User avatar
Drew Sebastino
Formerly Espozo
Posts: 3496
Joined: Mon Sep 15, 2014 4:35 pm
Location: Richmond, Virginia

Re: A simple sprite demo for teaching

Post by Drew Sebastino »

Yes. I'll look at main.s in a bit and tell you what to split.
User avatar
Drew Sebastino
Formerly Espozo
Posts: 3496
Joined: Mon Sep 15, 2014 4:35 pm
Location: Richmond, Virginia

Re: A simple sprite demo for teaching

Post by Drew Sebastino »

I stood looking at main.s and Walker.asm, wondering how you could make it better when I just realized the oblivious...

Walker looks a lot more organized because it has huge barricades like this to separate different parts of the code. (The parts you jmp and jsr to.)

Code: Select all

;==============================================================================
; main
;==============================================================================
Also, you need to change stuff like this:

Code: Select all

  cmp #28
  bcs notHitLeft
  lda #28
  sta player_xhi
  stz player_dxlo
  beq doneWallCollision
notHitLeft:
  cmp #212
  bcc notHitRight
  lda #211
  sta player_xhi
  stz player_dxlo
notHitRight:
doneWallCollision:
Into this:

Code: Select all

  cmp #28
  bcs notHitLeft
  lda #28
  sta player_xhi
  stz player_dxlo
  beq doneWallCollision

notHitLeft:
  cmp #212
  bcc notHitRight
  lda #211
  sta player_xhi
  stz player_dxlo

notHitRight:
doneWallCollision:
Why in the world do you even have two different places you can jump to if there's not a single thing in between them?

Also, you seem to have the code that moves the character and checks for collision smack dap in between the code that sets the BG mode and stuff (I think?) and some code that does something with the tile data. Walker kind of does this, but not quite as bad. I just think the main problem is that it is not a very clear where something ends and something starts. If you want to split the file, you could have all BG mode and BG tile stuff in a different file from the part where the controllers are read to make the character move, so it is a little more clear as to what is going on.

I might try to help you a bit on this latter, (I don't really want to use wla in its current buggy state, but I am.) but for now, you might just want to take a look at walker in the SNES starterkit.
User avatar
NovaSquirrel
Posts: 483
Joined: Fri Feb 27, 2009 2:35 pm
Location: Fort Wayne, Indiana
Contact:

Re: A simple sprite demo for teaching

Post by NovaSquirrel »

Espozo wrote:Why in the world do you even have two different places you can jump to if there's not a single thing in between them?
With two labels, you can put code between them later and it's still going to act correctly, and you don't need to remember to update the branches. If I had done that then Sliding Blaster wouldn't have ever had that crash bug.
User avatar
koitsu
Posts: 4201
Joined: Sun Sep 19, 2004 9:28 pm
Location: A world gone mad

Re: A simple sprite demo for teaching

Post by koitsu »

I will talk with tepples later about the "quality" of his code.

For Espozo: I would not bother spending any time analysing this code in comparison to your own. What I have been working on since last night is converting your code to use ca65 (including all necessary .exes, an updated .bat, etc.). I've done most of the work at this point, but as you can see from the other thread, I'm obviously running into problems with ca65 which are certainly user error on my part, and I cannot progress further until I can talk to someone who actually has very in-depth familiarity with ca65.
User avatar
thefox
Posts: 3134
Joined: Mon Jan 03, 2005 10:36 am
Location: 🇫🇮
Contact:

Re: A simple sprite demo for teaching

Post by thefox »

Espozo wrote:Why in the world do you even have two different places you can jump to if there's not a single thing in between them?
Got ninjaed by NovaSquirrel. Anyways, I do the same thing quite often, and it's to help with code clarity and maintainability.

Looks like there's a bug in that piece of code. beq doneWallCollision never jumps since A was loaded with 28, but the bcc notHitRight below it ends up picking up the slack. On top of that, I think it's a good practice to add a comment when you're purposefully using a conditional branch instruction for an unconditional branch, e.g. something like:

Code: Select all

bne doneWallCollision ; jumps always
...or just use BRA since we're on 65816 :)
Download STREEMERZ for NES from fauxgame.com! — Some other stuff I've done: fo.aspekt.fi
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

Thank you for taking the time to look at my code. Sometimes it's like pulling teeth to get a code review. Please bear with me while I try to understand the importance of each issue you uncovered.
Espozo wrote:Walker looks a lot more organized because it has huge barricades like this to separate different parts of the code. (The parts you jmp and jsr to.)
Ideally, that's what .proc is for. But if .proc isn't noticeable in the default settings of other users' existing favorite text editors, then I might need to put doc comments on all subroutines, even subroutines whose behavior is evident from the name and which takes no arguments. Here's what a doc comment from ppuclear.s looks like:

Code: Select all

;;
; Clears a nametable (2048 bytes) to a constant value.
; @param X starting address of nametable in VRAM (16-bit)
; @param Y value to write (16-bit)
Also, you need to change stuff like this:
My coding convention often includes a blank line after a "skip forward" type label, roughly corresponding to the End If of an If ... Then ... End If block. You are correct that I was missing a blank line.
Why in the world do you even have two different places you can jump to if there's not a single thing in between them?
Because the jumps are for different reasons, one for not having hit the right side wall, and one for being completely done with collision checks for this frame. It helps with inserting additional checks at the right place should they become needed.

And yes, I discovered the same bug that thefox mentioned (a never taken branch that was intended to be always taken), but my laptop's 2 1/2-year-old battery immediately dropped from 35% to like 8% before I could finish this post about having done so. I had changed a lda #$00 sta to stz because hey, 65816, but I forgot to change the following branch. Here's what it has become:

Code: Select all

  ; Test for collision with side walls
  ; (in actual games, this would involve collision with a tile map)
  cmp #28
  bcs notHitLeft
  lda #28
  sta player_xhi
  stz player_dxlo
  bra doneWallCollision
notHitLeft:

  cmp #212
  bcc notHitRight
  lda #211
  sta player_xhi
  stz player_dxlo
notHitRight:

  ; Additional checks for collision, if needed, would go here.
doneWallCollision:

Would this make more sense if I were to indent the "then clause" stuff?

Code: Select all

  ; Test for collision with side walls
  ; (in actual games, this would involve collision with a tile map)
  cmp #28
  bcs notHitLeft
    lda #28
    sta player_xhi
    stz player_dxlo
    bra doneWallCollision
  notHitLeft:

  cmp #212
  bcc notHitRight
    lda #211
    sta player_xhi
    stz player_dxlo
  notHitRight:

  ; Additional checks for collision, if needed, would go here.
doneWallCollision:

Also, you seem to have the code that moves the character and checks for collision smack dap in between the code that sets the BG mode and stuff (I think?) and some code that does something with the tile data.
Current order of subroutine:
Top level functions: nmi_handler, main
"Game logic" subroutine: move_player
Graphics subroutines: draw_player_sprite, draw_bg
Graphics data
I just think the main problem is that it is not a very clear where something ends and something starts.
In my ca65 coding convention, everything from .proc to .endproc can be pulled out and put into another file.

Let me know what other improvements I could add to make this more understandable, and then I can backport them to the NES version.
User avatar
Drew Sebastino
Formerly Espozo
Posts: 3496
Joined: Mon Sep 15, 2014 4:35 pm
Location: Richmond, Virginia

Re: A simple sprite demo for teaching

Post by Drew Sebastino »

tepples wrote:Would this make more sense if I were to indent the "then clause" stuff?
Yes.
tepples wrote:Current order of subroutine:Top level functions: nmi_handler, main"Game logic" subroutine: move_playerGraphics subroutines: draw_player_sprite, draw_bgGraphics data
I know this may sound a bit crazy to you, but another thing that would make the code a lot more "comprehendible" would be if you actually separated these into separate files.

Sorry if I seem a bit like a jerk...
tepples
Posts: 22708
Joined: Sun Sep 19, 2004 11:12 pm
Location: NE Indiana, USA (NTSC)
Contact:

Re: A simple sprite demo for teaching

Post by tepples »

Are you talking one subroutine per file? I've seen that in libraries to allow the linker to remove unused subroutines. If not, then what level of granularity between that and the current state of the demo would be best?
User avatar
Drew Sebastino
Formerly Espozo
Posts: 3496
Joined: Mon Sep 15, 2014 4:35 pm
Location: Richmond, Virginia

Re: A simple sprite demo for teaching

Post by Drew Sebastino »

tepples wrote:Are you talking one subroutine per file? I've seen that in libraries to allow the linker to remove unused subroutines. If not, then what level of granularity between that and the current state of the demo would be best?
What? You don't have to put everything in a separate file, but maybe try taking the largest subroutine (Isn't it move_player?) and putting it by itself. It's also usually nice to have graphics have a separate file, because otherwise, you'll end up with a huge main file.
Post Reply