Nestopia and Ninja Gaiden/Burai Fighter

Discuss emulation of the Nintendo Entertainment System and Famicom.

Moderator: Moderators

User avatar
*Spitfire_NES*
Posts: 306
Joined: Fri May 21, 2010 4:10 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by *Spitfire_NES* »

Thank you for your valuable input perilsensitive and also thank you for posting the fix for burai fighter as well. :beer:

I sent you a pm by the way as i am curious about something else you said regarding ninja gaiden and figured it would be easier to pm you as to not derail the thread.

I would have never guessed in a million years that the problem with ninja gaiden (U) would be something like that. Thats why im a noob though, and its great to see the community come together and share fixes, information and the like.

Ive learned a lot from everyone and appreciate anyone who has taken the time to post their thoughts on anything i have ever asked. :D
User avatar
*Spitfire_NES*
Posts: 306
Joined: Fri May 21, 2010 4:10 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by *Spitfire_NES* »

sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?
zxbdragon
Posts: 498
Joined: Mon Dec 12, 2011 8:15 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by zxbdragon »

*Spitfire_NES* wrote:sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?
Integration code

if (!(regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) ||
(scanline == SCANLINE_VBLANK)) {
UpdateAddressLine(scroll.address & 0x3fff);
}

to UpdateScrollAddressLine;
perilsensitive
Posts: 13
Joined: Mon Jul 15, 2013 3:35 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by perilsensitive »

*Spitfire_NES* wrote:sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?
I actually eliminated UpdateScrollAddressLine() in my patch because it doesn't change the value of io.address, which is the value currently on the address bus. It really should though, as the actual bus value does change according to Visual 2C02. Since UpdateAddressLine already does that (and it seems the point of UpdateScrollAddressLine() was to *not* update the bus), it didn't make sense to leave it in.

As for the shaky status bar in Safari in Letterland, the fix James posted requires that the PPU never put addresses >= $3000 on the address bus (specifically, he forces A12 to 0 for addresses in that range before putting them on the bus, preventing the counter from being clocked), but that doesn't seem to match what Visual 2C02 does so I didn't include that change in my patch. The point of my patch was to show that the problem with Burai Fighter had a different (although somewhat related) root cause, the fix for which didn't require any special handling of $3000-$3FFF.

You could integrate the two patches by making a similar change to the one James made in his post, but instead of doing it to UpdateScrollAddressLine() (which no longer exists) do it directly to the address passed to UpdateAddressLine() in the implementations of $2006 and $2007. I'm too lazy to do this myself though. :-)

It may (or may not) be worth noting that Safari in Letterland uses an MC-ACC, which James has shown has similar but different IRQ behavior to that of the MMC3. The difference James documented in that thread doesn't really make a difference for Safari in Letterland, but it makes me wonder if there are other differences we're missing that do. :-/
zxbdragon
Posts: 498
Joined: Mon Dec 12, 2011 8:15 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by zxbdragon »

in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}
[[)$6TQ]WWWJR@[G}554ESY.jpg
[[)$6TQ]WWWJR@[G}554ESY.jpg (137.24 KiB) Viewed 4769 times
The need to observe and test
perilsensitive
Posts: 13
Joined: Mon Jul 15, 2013 3:35 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by perilsensitive »

zxbdragon wrote:in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}
This won't work; it prevents the attribute glitch but breaks DMC DMA timing. For example, the sprdma_and_dmc_dma test roms hang before producing output with this change, which makes DMA take one extra cycle. blargg demonstrated that at most DMC DMA will steal 3 cycles if it falls on a write cycle and 4 if it falls on a read. The first 2 cycles on a write or 3 cycles on a read are where the CPU is paused, doing nothing. The last cycle is the actual DMA transfer; the CPU is still paused but the DMA unit is reading from memory. Your change adds an extra cycle before the transfer starts, making DMA now steal 4 cycles on a write and 5 cycles on a read. This may break games that require precise DMC DMA timing.
zxbdragon
Posts: 498
Joined: Mon Dec 12, 2011 8:15 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by zxbdragon »

perilsensitive wrote:
zxbdragon wrote:in NstApu.cpp 2138
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 2 : 3) );
}
to
if (!readAddress)
{
cpu.StealCycles( cpu.GetClock(cpu.IsWriteCycle(clock) ? 3 : 4) );
}
This won't work; it prevents the attribute glitch but breaks DMC DMA timing. For example, the sprdma_and_dmc_dma test roms hang before producing output with this change, which makes DMA take one extra cycle. blargg demonstrated that at most DMC DMA will steal 3 cycles if it falls on a write cycle and 4 if it falls on a read. The first 2 cycles on a write or 3 cycles on a read are where the CPU is paused, doing nothing. The last cycle is the actual DMA transfer; the CPU is still paused but the DMA unit is reading from memory. Your change adds an extra cycle before the transfer starts, making DMA now steal 4 cycles on a write and 5 cycles on a read. This may break games that require precise DMC DMA timing.
You find a better solution ?
User avatar
James
Posts: 431
Joined: Sat Jan 22, 2005 8:51 am
Location: Chicago, IL
Contact:

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by James »

perilsensitive wrote:I tested this in Visual 2C02...while rendering the bus is controlled by the rendering logic, and therefore not directly affected by the change to loopy_v)
Nice job digging into this!
perilsensitive wrote:It may (or may not) be worth noting that Safari in Letterland uses an MC-ACC, which James has shown has similar but different IRQ behavior to that of the MMC3. The difference James documented in that thread doesn't really make a difference for Safari in Letterland, but it makes me wonder if there are other differences we're missing that do. :-/
I played around with this a bit, but didn't have much luck either. It'll be interesting to find out what the issue is here... I'll try to take a look at it on the hardware sometime soon.
get nemulator
http://nemulator.com
perilsensitive
Posts: 13
Joined: Mon Jul 15, 2013 3:35 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by perilsensitive »

zxbdragon wrote:You find a better solution ?
I don't have a solution yet. Every emulator that implements these extra reads caused by DMA has similar glitches, so *nobody* has a solution yet. You could disable the extra reads altogether, although this would still be a hack. No games depend on this behavior, and many emulators simply don't implement it at all (FCEUX, for example). It is of interest to NES developers though, especially when it causes extra reads of the controller ports.

You could do something like this in NstApu.cpp to disable the reads if the glitch really bothers you that much:

Code: Select all

diff --git a/source/core/NstApu.cpp b/source/core/NstApu.cpp
index cf1f3df..cd14e6c 100644
--- a/source/core/NstApu.cpp
+++ b/source/core/NstApu.cpp
@@ -2137,19 +2137,19 @@ namespace Nes
                        {
                                cpu.StealCycles( cpu.GetClock(3) );
                        }
-                       else
-                       {
-                               NST_DEBUG_MSG("DMA/Read conflict!");
+                       // else
+                       // {
+                       //      NST_DEBUG_MSG("DMA/Read conflict!");
 
-                               cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
 
-                               if ((readAddress & 0xF000) != 0x4000)
-                                       cpu.Peek( readAddress );
+                       //      if ((readAddress & 0xF000) != 0x4000)
+                       //              cpu.Peek( readAddress );
 
-                               cpu.StealCycles( cpu.GetClock(1) );
-                               cpu.Peek( readAddress );
-                               cpu.StealCycles( cpu.GetClock(1) );
-                       }
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.Peek( readAddress );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       // }
 
                        dma.buffer = cpu.Peek( dma.address );
                        cpu.StealCycles( cpu.GetClock() );
User avatar
*Spitfire_NES*
Posts: 306
Joined: Fri May 21, 2010 4:10 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by *Spitfire_NES* »

zxbdragon wrote:
*Spitfire_NES* wrote:sorry for the double post just wanted to add this real quick. I merged in your fix for burai fighter, but i noticed this overwrites james fix for mickey's in nstppu.cpp he shared a while back.

NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
io.line.Toggle( scroll.address & 0x3FFF, cpu.GetCycles() );
}


to


NST_FORCE_INLINE void Ppu::UpdateScrollAddressLine()
{
if (io.line)
{
int a12_mask = ~((scroll.address & 0x2000) >> 1);
io.line.Toggle( (scroll.address & a12_mask) & 0x3FFF, cpu.GetCycles() );
}
}


This part was overwritten so mickeys return back to the shaky status bar. Is there a way to put this back in for both games to coexist peacefully?
Integration code

if (!(regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) ||
(scanline == SCANLINE_VBLANK)) {
UpdateAddressLine(scroll.address & 0x3fff);
}

to UpdateScrollAddressLine;
I tried your fix zxbdragon to get the mickeys fix reverted back to this:

if ((regs.ctrl[1] & Regs::CTRL1_BG_SP_ENABLED) && !(data & Regs::CTRL1_BG_SP_ENABLED))
UpdateScrollAddressLine(scroll.address & 0x3fff);

and mickeys did not revert back. What did you do exactly and what is the best way to integrate james fix and your patch?

Thanks again for all your contributions to this. Looks like we are all learning something here lol.

As for ninja gaiden ill add this in and give it a whirl and see where it goes. I dont think anything will get broken in the process as you seem to know your ins and outs far better than me. :D :beer: :beer:
User avatar
Zepper
Formerly Fx3
Posts: 3262
Joined: Fri Nov 12, 2004 4:59 pm
Location: Brazil
Contact:

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by Zepper »

@perilsensitive

Please, you must rephrase what you wrote about the IRQ fix. It's quite confusing to me, since you mixed it with tepple's question.
perilsensitive
Posts: 13
Joined: Mon Jul 15, 2013 3:35 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by perilsensitive »

I've been looking into the problem with Safari in Letterland a bit more after briefly discussing the issue with James via PM, and I have data explaining why the shaking happens, as well as a possible theory for what the actual fix is. Of course, it's possible that somebody has already tested this aspect of the MC-ACC and my theory is wrong, but I can't think of a better explanation at the moment. :-/

The status bar shakes due to the IRQ firing early on some frames. It is supposed to fire on scanline 205, but occasionally fires on scanline 204 (James pointed out some $2006 updates that occur when rendering; these happen one scanline after the IRQ). It fires on scanline 204 instead due to an extra clock occurring around cycle 300 on scanline 188. That clock happens because the game disables rendering around cycle 276 (putting vramaddr_v onto the address bus), then writes a palette address to $2006 around cycle 300 (putting the $3Fxx address onto the bus). If the vertical scroll value was even at the time rendering was disabled, this triggers an A12 rise. The difference in PPU clocks between when A12 falls and then rises again is between 24 and 30 clocks, clearly more than 15, so the IRQ counter gets clocked.

The only way around this (assuming that the PPU does put the palette address onto the bus and that nothing else is being emulated incorrectly), is to require more time between A12 rises in order to clock the counter. This doesn't seem unreasonable, especially since we already know that the MC-ACC doesn't *quite* behave like an MMC3. The required time between rises must be at least the same as required by the MMC3, but there's no reason it couldn't be greater. I tried various values and found that a minimum of 39 PPU cycles (or 13 M2 cycles, since the cart doesn't have access to the PPU clock signal) are required to fix the shaky status bar. The actual required time between rises may be larger than this, but there's no way to know what it is without testing an actual MC-ACC.

I've tested this theory in my emulator as well as Nestopia, and the shakes completely disappear (and the status bar looks correct: no scanline missing from the middle). I don't have a clean patch for Nestopia yet though, and simply changing the value of CLOCK_FILTER in NstBoardMmc3.hpp (as I did to test this theory) will definitely break actual MMC3 games. This is all purely academic if my theory is wrong though, so I'm hesitant to create a patch until this is confirmed (if it is confirmed). :-)
perilsensitive
Posts: 13
Joined: Mon Jul 15, 2013 3:35 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by perilsensitive »

Zepper wrote:@perilsensitive

Please, you must rephrase what you wrote about the IRQ fix. It's quite confusing to me, since you mixed it with tepple's question.
I will reply via PM to avoid repeating myself in the thread. :-)
User avatar
*Spitfire_NES*
Posts: 306
Joined: Fri May 21, 2010 4:10 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by *Spitfire_NES* »

hey peril, in the meantime what is the easiest way to merge the 2 patches together until a proper fix is found if you dont mind me asking? :)
zxbdragon
Posts: 498
Joined: Mon Dec 12, 2011 8:15 pm

Re: Nestopia and Ninja Gaiden/Burai Fighter

Post by zxbdragon »

perilsensitive wrote:
zxbdragon wrote:You find a better solution ?
I don't have a solution yet. Every emulator that implements these extra reads caused by DMA has similar glitches, so *nobody* has a solution yet. You could disable the extra reads altogether, although this would still be a hack. No games depend on this behavior, and many emulators simply don't implement it at all (FCEUX, for example). It is of interest to NES developers though, especially when it causes extra reads of the controller ports.

You could do something like this in NstApu.cpp to disable the reads if the glitch really bothers you that much:

Code: Select all

diff --git a/source/core/NstApu.cpp b/source/core/NstApu.cpp
index cf1f3df..cd14e6c 100644
--- a/source/core/NstApu.cpp
+++ b/source/core/NstApu.cpp
@@ -2137,19 +2137,19 @@ namespace Nes
                        {
                                cpu.StealCycles( cpu.GetClock(3) );
                        }
-                       else
-                       {
-                               NST_DEBUG_MSG("DMA/Read conflict!");
+                       // else
+                       // {
+                       //      NST_DEBUG_MSG("DMA/Read conflict!");
 
-                               cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
 
-                               if ((readAddress & 0xF000) != 0x4000)
-                                       cpu.Peek( readAddress );
+                       //      if ((readAddress & 0xF000) != 0x4000)
+                       //              cpu.Peek( readAddress );
 
-                               cpu.StealCycles( cpu.GetClock(1) );
-                               cpu.Peek( readAddress );
-                               cpu.StealCycles( cpu.GetClock(1) );
-                       }
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       //      cpu.Peek( readAddress );
+                       //      cpu.StealCycles( cpu.GetClock(1) );
+                       // }
 
                        dma.buffer = cpu.Peek( dma.address );
                        cpu.StealCycles( cpu.GetClock() );
thank you,I know how to solve the!
Post Reply