It is currently Sat May 27, 2017 5:51 pm

All times are UTC - 7 hours



Forum rules


Related:



Post new topic Reply to topic  [ 58 posts ]  Go to page Previous  1, 2, 3, 4  Next
Author Message
PostPosted: Tue Oct 28, 2014 7:34 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 18361
Location: NE Indiana, USA (NTSC)
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.


Top
 Profile  
 
PostPosted: Thu Oct 30, 2014 3:38 pm 
Offline
User avatar

Joined: Wed Feb 10, 2010 4:00 pm
Posts: 82
Location: Indianapolis, IN
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.


Top
 Profile  
 
PostPosted: Wed Jan 28, 2015 2:17 pm 
Offline

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


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 2:12 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 18361
Location: NE Indiana, USA (NTSC)
So it appears my code is unreadable.

In this post, 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?

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".

Quote:
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?


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 2:16 pm 
Offline
User avatar

Joined: Mon Sep 15, 2014 4:35 pm
Posts: 2964
Location: Nacogdoches, Texas
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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 3:19 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 18361
Location: NE Indiana, USA (NTSC)
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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 3:25 pm 
Offline
User avatar

Joined: Mon Sep 15, 2014 4:35 pm
Posts: 2964
Location: Nacogdoches, Texas
Yes. I'll look at main.s in a bit and tell you what to split.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 3:58 pm 
Offline
User avatar

Joined: Mon Sep 15, 2014 4:35 pm
Posts: 2964
Location: Nacogdoches, Texas
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:
;==============================================================================
; main
;==============================================================================


Also, you need to change stuff like this:

Code:
  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:
  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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 4:55 pm 
Offline
User avatar

Joined: Fri Feb 27, 2009 2:35 pm
Posts: 174
Location: Fort Wayne, Indiana
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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 5:02 pm 
Offline
User avatar

Joined: Sun Sep 19, 2004 9:28 pm
Posts: 3192
Location: Mountain View, CA, USA
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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 5:04 pm 
Offline
User avatar

Joined: Mon Jan 03, 2005 10:36 am
Posts: 2816
Location: Tampere, Finland
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:
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: kkfos.aspekt.fi


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 6:04 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 18361
Location: NE Indiana, USA (NTSC)
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:
;;
; 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)


Quote:
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.

Quote:
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:
  ; 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:
  ; 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:



Quote:
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

Quote:
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.


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 6:37 pm 
Offline
User avatar

Joined: Mon Sep 15, 2014 4:35 pm
Posts: 2964
Location: Nacogdoches, Texas
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...


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 8:44 pm 
Offline

Joined: Sun Sep 19, 2004 11:12 pm
Posts: 18361
Location: NE Indiana, USA (NTSC)
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?


Top
 Profile  
 
PostPosted: Sat Jan 31, 2015 8:49 pm 
Offline
User avatar

Joined: Mon Sep 15, 2014 4:35 pm
Posts: 2964
Location: Nacogdoches, Texas
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.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 58 posts ]  Go to page Previous  1, 2, 3, 4  Next

All times are UTC - 7 hours


Who is online

Users browsing this forum: No registered users and 3 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group