It is currently Tue Sep 18, 2018 9:45 pm

All times are UTC - 7 hours





Post new topic Reply to topic  [ 153 posts ]  Go to page Previous  1 ... 7, 8, 9, 10, 11  Next
Author Message
PostPosted: Thu Aug 30, 2018 6:41 pm 
Offline
User avatar

Joined: Sun Jan 22, 2012 12:03 pm
Posts: 6801
Location: Canada
Sour wrote:
rainwarrior wrote:
Also, not related to this, but I believe the 5B frequencies are off by 1?
Are you asking because you noticed the difference by listening to it, or because you checked the code?
There is in fact a very suspicious looking +1 here: https://github.com/SourMesen/Mesen/blob ... udio.h#L58
Not sure why it's there, but that would probably be the cause?

I noticed the difference audibly while working on a 5B audio test, but I also checked with a recorded output to make sure it wasn't just the dynamic pitch synchronization thing.

I didn't analyize your code, but that +1 in your code implementation is probably the cause. A weird thing with 5B is that the "0" value seems to give a division by zero in the frequency formula, so maybe that creates a temptation to "fix" this with +1. (I'm not sure whether the chip halts at pitch 0 or what, but pitch 1 is already above audible at 55kHz.)

Sour wrote:
samophlange wrote:
Yeah, I'd like to request the same thing! I run in to this a lot when doing large scale refactors or setting up little test projects.
The "Break on CPU crash" option I mentioned should work - let me know if it doesn't though.

It works but you have to set it up before you try to run the ROM, because the option you need to set is inside the debugger that you can't open without a running ROM.


Top
 Profile  
 
PostPosted: Thu Aug 30, 2018 6:47 pm 
Offline
User avatar

Joined: Tue Jun 24, 2008 8:38 pm
Posts: 1937
Location: Fukuoka, Japan
I had a similar issue with one of my bug. My workaround was to start a different rom, pause it, start the one which fail and the new rom would start in paused mode (maybe by design, not sure). Then after that I would be able to set a breakpoint at the desired location.

It would be useful to have a mode in preference to not auto-start like nintendulor F2. It would be quite useful for those case.


Top
 Profile  
 
PostPosted: Thu Aug 30, 2018 7:03 pm 
Offline
User avatar

Joined: Sun Jan 22, 2012 12:03 pm
Posts: 6801
Location: Canada
I think the real problem is just that the halt instruction boots you from the game and unloads it. That's kind of weird in itself. The cartridge doesn't fly out of the NES when that instruction is hit.

Displaying a message that the game has crashed is cool, but I think it would make a lot more sense if the emulator just paused or halted but left you with the game, giving you the option to reset.


Top
 Profile  
 
PostPosted: Thu Aug 30, 2018 7:17 pm 
Offline
User avatar

Joined: Tue Jun 24, 2008 8:38 pm
Posts: 1937
Location: Fukuoka, Japan
Just the image of the cartridge flying out of the nes: Attack of the killer nintendo :lol:

Now on a serious note, yes, to stop and show the debugger where the program halted could help but sometime it could be that it just went on a wrong address and ended up in data. I don't know if there is a stack of the last commands before it failed in that case. But still, that is better than just stopping the emulator from a developer point of view. A user would just need the emulator to stop like it is doing at the moment.


Top
 Profile  
 
PostPosted: Thu Aug 30, 2018 7:43 pm 
Offline

Joined: Sun Feb 07, 2016 6:16 pm
Posts: 515
To be honest, the whole "close the game on crash" has been pretty annoying on my end too, given how often I end up testing/fixing games that crash due to bugs in obscure mappers, etc. It was originally meant to be a user-friendly way to handle CPU crashes for end users, but for people trying to debug ROMs, it has the opposite effect.

I changed it so that it no longer does this when Developer Mode is enabled, which makes a lot of sense, imo. In Dev Mode, it will still display the "cpu crashed" error (once per reset/power cycle), but the cpu will keep running as per usual afterwards.

The 5B audio should be fixed now, too.


Top
 Profile  
 
PostPosted: Fri Aug 31, 2018 10:55 am 
Offline

Joined: Tue Aug 28, 2018 8:54 am
Posts: 18
Location: Edmonton, Canada
I am not sure if it was discussed before or not. But when I use ca65, I use anonymous labels for local loops, which are not exported as symbols. Can the debugger auto assign labels to those jumps if they are within the same procedure?

So instead of
Code:
8054  LDA Pallete,X           
8055  STA PpuData_2007         
8056  INX                     
8057  CPX #$20                 
8058  BNE $8054


Display something like

Code:
@label213:           
8054  LDA Sprites,X           
8055  STA OamData,X           
8056  INX                     
8057  CPX #$10                 
8058  BNE @label213         


I know I can double click on the address, but it would be nice to visually find where it loops to without scrolling jumps or looking trough addresses.


Top
 Profile  
 
PostPosted: Fri Aug 31, 2018 11:43 am 
Offline
User avatar

Joined: Sun Jan 22, 2012 12:03 pm
Posts: 6801
Location: Canada
The debug information from ca65 unfortunately doesn't have any output for anonymous labels.

However, it might be nice to have any branch within view (or maybe within 128 bytes of in view) to a location without a label automatically generate a label there? That'd be useful even when not using ca65.

da65 has a convention for automatically naming labels by their address, i.e. "LFF35:" would go to $FF35. The specific name is probably less important than just the visual break you get for a label.


Top
 Profile  
 
PostPosted: Fri Aug 31, 2018 5:29 pm 
Offline

Joined: Sun Feb 07, 2016 6:16 pm
Posts: 515
Dynamically generating labels around the current view would be sort of hard - the disassembly is generated when the debugger updates (e.g when stepping through the code), but not when scrolling (everything is already disassembled ahead of time at that point). It would also be a bit tricky to handle scrolling properly if labels appear/disappear as the user scrolls.

I've been thinking about it for a little bit and there's a couple of options I could probably do:
A) Make the disassembler keep track of all jump destinations it finds while disassembling and add fake temporary "labels" for them, which might look something like this:
Code:
     $8054:           
8054   LDA Sprites,X           
8055   STA OamData,X           
8056   INX                     
8057   CPX #$10                 
8058   BNE $8054
These wouldn't show up in the label list, though (as they wouldn't be real labels). This would technically work for any type of memory (RAM, ROM), but when disassembling unverified bytes you could end up with random labels all over (so it would probably require ignoring jump from inside unverified code to verified code, etc.)

B) Create proper labels for all of PRG ROM as the code runs, e.g automatically create a real label whenever a label-less jump target is found (if the target is in PRG ROM). This would be a lot less of a hack, code-wise, and has the benefit that the labels would be permanent, but maybe it would result in too many labels? This would also be limited to PRG ROM code that already been executed by the CPU. In this case you might end up with this:
Code:
     L38054:
8054   LDA Sprites,X           
8055   STA OamData,X           
8056   INX                     
8057   CPX #$10                 
8058   BNE L38054
(assuming that $8054 in CPU memory maps to $38054 in the PRG ROM, for example)

Personally I'm more inclined to go with B) - it would be a lot more straightforward to implement, and would probably be the best when trying to reverse-engineer/annotate a ROM's code. The main downsides would be that it wouldn't work for FDS games, or when games execute code from RAM.

If B) doesn't feel like it would be useful, though, I can try to figure out something else/better.


Top
 Profile  
 
PostPosted: Fri Aug 31, 2018 9:32 pm 
Offline

Joined: Tue Aug 28, 2018 8:54 am
Posts: 18
Location: Edmonton, Canada
Both versions look great. Even just having this visual clue, will help a lot. Usually you have 2-3 branches around and even if they overlay, you can just remember the numbers.

Just a random thought after poking IDA today. Can we convert these pseudo/auto labels into the real labels and give them names? It is not necessary that I would use it, but it may help people who study old games.


Top
 Profile  
 
PostPosted: Sat Sep 01, 2018 4:43 pm 
Offline

Joined: Sun Feb 07, 2016 6:16 pm
Posts: 515
A quick test gives something like this:
Attachment:
autolabels.png
autolabels.png [ 26.83 KiB | Viewed 749 times ]

These are regular labels that get auto-generated by the debugger as it runs, so yes, they can be renamed, etc.
As I have it now, they can't really be erased while the option to create them is enabled, though. (they will get recreated after being deleted).

The labels are only created once the CPU reaches the branch instruction (otherwise there is no guarantee that the target will be the same once the instruction is reached, because of bank switching, etc.). In terms of integration with CA65, that means they would get deleted every time you power cycle or reload the ROM, but get recreated each time as you run the code again.

I'm thinking of adding 2 separate options to control this:
-1 option to control if labels should be auto-generated for PRG ROM jumps (behavior would be identical to 0.9.6 when disabled)
-1 option on the label list to "Show generated labels", which would show/hide those labels in the label list (but they would still be used in the disassembly, whether or not they are shown in the list). This would help keep the list shorter and easier to work with (and the list would get populated as you rename the labels manually)


Top
 Profile  
 
PostPosted: Tue Sep 04, 2018 12:39 pm 
Offline

Joined: Tue Aug 28, 2018 8:54 am
Posts: 18
Location: Edmonton, Canada
Sour wrote:
that means they would get deleted every time you power cycle or reload the ROM, but get recreated each time as you run the code again.


Is it possible to keep the generated/renamed labels in the "Workspace.xml" file along with hash of the rom, and leave them in place if hash is not changed?

If user study the existing game rom, it may take more than one reload to study code, label procedures (to remember what they do later).

I am no NES developer (or yet at least), but I'm looking from the point of my current development job. If I understand correctly, unless jump address is within ram range ($000-$7FF) and rom has not changed, there is no reason to forget labels on reset/power cycle. I may be wrong here though.


Top
 Profile  
 
PostPosted: Tue Sep 04, 2018 4:09 pm 
Offline

Joined: Sun Feb 07, 2016 6:16 pm
Posts: 515
yaros wrote:
Is it possible to keep the generated/renamed labels in the "Workspace.xml" file along with hash of the rom, and leave them in place if hash is not changed?
This is already the case, assuming you're not loading a .dbg file. Every time a .dbg file is loaded, it resets all existing labels, which would include these auto-generated labels (which is what I was referring to in my previous post)

Saving the PRG ROM's CRC in the workspace.xml file and auto-resetting the generated labels when that changes shouldn't be all that hard, though (I've also been meaning to add a similar option with regards to the CDL files for a while now, but never got around to it).

I'll try to finish this feature up at some point this week if I can.


Top
 Profile  
 
PostPosted: Tue Sep 04, 2018 6:58 pm 
Offline

Joined: Tue Aug 28, 2018 8:54 am
Posts: 18
Location: Edmonton, Canada
Sour wrote:
Every time a .dbg file is loaded, it resets all existing labels, which would include these auto-generated labels (which is what I was referring to in my previous post)


Oh, I see what happens.

I want to explain though. I am not trying to push this idea as it is must be/must have, you are the author and maintainer in the end of the day. I haven't touched assembler in ages and frankly didn't even finish my "pong". Those are the things I just noticed poking the Mesen debugger (which is honestly already amazing) and after seeing this thread I though to chime in.


Top
 Profile  
 
PostPosted: Sat Sep 08, 2018 3:37 am 
Offline
User avatar

Joined: Sun Sep 19, 2004 9:28 pm
Posts: 3589
Location: Mountain View, CA
Some minor bug reports (mainly cosmetical):

Debug -> Memory Tools -> Memory Viewer tab. The "View" pulldown (for "CPU Memory", "PPU Memory", etc.) is not wide enough to display the full name of some entries, particularly "Sprite (OAM) RAM" (shown as Sprite (OAM) RA) and "Secondary OAM RAM" (shown as Secondary OAM). I know this varies per OS/theme/DPI/etc. a bit, but it looks like there's a lot of space available to the right of that pulldown..... ;-)

Debug -> Memory Tools -> Memory Viewer tab. The latter two "View" pulldowns should be syntactically similar; "Sprite (OAM) RAM" and "Secondary OAM RAM" seem weird (particularly the inconsistent use of parenthesis). I know the secondary OAM table is unique/sort of an oddity, but still. Might I suggest alternatives:

- "Sprite/OAM RAM" + "Secondary OAM RAM"
- "Primary OAM RAM" + "Secondary OAM RAM"
- "Primary (OAM) RAM" + "Secondary (OAM) RAM"
- "Primary Sprite RAM" + "Secondary Sprite RAM"

If this conflicts with existing naming schemes you have throughout the program and would have to do a global replace + ensure all the fields are wide enough for changes, don't bother.

Debug -> Memory Tools -> Memory Viewer tab. Why is the "View" pulldown in a completely different theme/thing than the Access Counters "View" pulldown and several others under Memory Tools? Maybe it's because it has horizontal section dividers?

Debug -> APU Viewer -> there are inconsistencies in the spacing/alignment of some of these fields:

- Square 1 and Square 2: Sweep Unit "Period" and "Shift" texts are slightly indented compared to the checkboxes/states above
- Triangle: "Period", "Timer", "Frequency", and "Output Volume" are slightly indented. This may be because
- Triangle: Change "Sequence Position" to "Sequence Pos" or "Sequence Pos." so that the text fits within the space available and doesn't look "too close" to the actual field data

- Square 1, Square 2, and Noise: Envelope checkboxes/states ("Start Flag", etc.) need to be shifted to the right a little bit to be consistent with the rest of the other sections/UI
- Square 1, Square 2, and Noise: "Counter", "Divider", and "Volume" need to be aligned properly with above checkboxes/states once fixed

- DMC: "Bytes remaining" should have a capital "R"
- DMC: "Sample Rate" "Hz" suffix should be moved over to the left a little bit (3-4 pixels? See Square 1 & 2, or Triangle for examples)
- Frame Counter: "Current Step" need to be aligned properly with above checkboxes/states (moved to left a couple pixels)
- Frame Counter: "5-step Mode" should have a capital "M"

- Channel Control: horizontal spacing between "columns" of checkboxes should be more consistent/evenly distributed
- Channel Control: "Namco" should be "N163", "Sunsoft" should be "5B" (rest of UI uses chip names, not company names, ex. MMC5, VRC6)

For the APU Viewer, I've attached a screenshot with some vertical alignment lines to give you an idea of what I'm talking about.

Some of my above items might seem inconsistent in their recommendations, but the overall goal is: make all the stuff consistent. Haha :D

Not bragging, but yeah, I have an incredible eye for this sort of thing. I can often tell if something is 1 pixel off on a 24" @ 1920x1200 display; I've been called nuts (and OCD) more than once. ;-) The amusing part: I have *horrible* vision -- I got checked last week: nearsighted, 20/200 (not a typo).

P.S. -- Event Viewer is absolutely mind-blowing. When I discovered this (it's been a while since I've used Mesen), I sat here with my mouth agape.

P.P.S. -- If you would prefer this be filed in GitHub Issues, happy to do so (I just now noticed the "Report a bug" pulldown).


Attachments:
ss.png
ss.png [ 63.76 KiB | Viewed 529 times ]
Top
 Profile  
 
PostPosted: Sat Sep 08, 2018 8:00 am 
Offline

Joined: Sun Feb 07, 2016 6:16 pm
Posts: 515
Thanks! Most of these should be fixed now.

koitsu wrote:
Debug -> Memory Tools -> Memory Viewer tab. Why is the "View" pulldown in a completely different theme/thing than the Access Counters "View" pulldown and several others under Memory Tools? Maybe it's because it has horizontal section dividers?
Yes, I can't really do much about it. I turned on "owner draw" for the dropdown list items to display proper separators (as opposed to just an empty option filled with dashes), but the dropdown's styling, for some reason (on Win 10 this is limited to the background color being different, but I think Win 7 makes it more obvious?). IIRC, I tried fixing this when I first implemented the dropdown, but could not find a way to manually draw the contents of the dropdown without altering the dropdown's style.

koitsu wrote:
P.S. -- Event Viewer is absolutely mind-blowing. When I discovered this (it's been a while since I've used Mesen), I sat here with my mouth agape.
Most of the credit for the event viewer goes to Bananmos for making the request in the first place :p


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 153 posts ]  Go to page Previous  1 ... 7, 8, 9, 10, 11  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