search

0ldsk00l / nestopia

Cross-platform Nestopia emulator core with a GUI
http://0ldsk00l.ca/nestopia
GNU General Public License v2.0
701 stars 168 forks source link

Mapper 22/23 mirroring selection inaccurately emulated #172

Closed koitsu closed 8 years ago

koitsu commented 8 years ago

Issue: Mapper 22 (VRC2a) and 23 (VRC2b) mirroring selection ($9000-9003) is inaccurately emulated.

Details: For some time, VRC2a/2b reverse-engineering documentation has been incorrect (docs by Goroh and Disch are incorrect); said docs stated VRC2a/2b supported 4 different modes of mirroring (H, V, 1-screen (lower), 1-screen (upper)), similar to VRC4.

This has been fully debunked through several means (incl. but not limited to translation of official Konami VRC2 docs). It's been determined that VRC2a/2b only implements H/V mirroring, controlled via bit 0; there is no 1-screen support in VRC2a/2b.

Inaccurate emulation of this register manifests itself badly in Konami's Wai Wai World (mapper 23), where in late stages of the game -- particularly parts of the overhead vertically-scrolling shooter stage, and the staff roll/credits -- graphics appear corrupted.

Fix: patch not available, but the proper solution is to only honour bit 0 of writes to $9000-9003 registers (0=V, 1=H).

Save states for testing (.zip file):

References:

lidnariq commented 8 years ago

Please note that I already tried switching the line in KonamiVrc2.cpp from Map( 0x9000U, 0x9FFFU, NMT_SWAP_VH01 ); to Map( 0x9000U, 0x9FFFU, NMT_SWAP_VH ); and it tickled some unrelated mirroring bug.

dragon2snow commented 8 years ago

2016-04-06_16-23-14 2016-04-06_16-25-51

You to test all VRC2 game!

dragon2snow commented 8 years ago

VRC2 is h,v,0,1,but wai wai world ...

koitsu commented 8 years ago

We have confirmed repeatedly (on actual hardware, and per official Konami documentation) that VRC2 does not support single-screen mirroring. Please see the References links for details, especially the nesdev forum thread, which includes testing on actual hardware.

If you have evidence of one-screen mirroring on an actual VRC2 game, we'd love to know it. All that's been shown so far is anecdotal classic documentation that seems to be based off of VRC4 (which does support H/V/1-screen).

dragon2snow commented 8 years ago

try my fixed.

koitsu commented 8 years ago

The 0xFF comparison "quirk" has already been discussed in the nesdev thread by the same person as here (lidnariq), which is what FCEUX does. And it was agreed that it's an incorrect hack. We even tried to track down the history/reason for this hack, but it was so long ago that it wasn't in FCEUX's commit history. It was concluded that the 0xFF comparison hack was chosen a long time ago to make Wai Wai World work, because the emulator was developed with the belief (based on docs from Firebug and Goroh) that VRC2a/2b support one-screen.

However, the core of the matter is that VRC2a/VRC2b, from testing on actual hardware, shows that there is no one-screen mirroring support, only H/V. In other words: only bit 0 of the mirror toggle registers is honoured on mappers 22 and 23.

If you have knowledge of real VRC2a/VRC2b games that have VRC2 chips that do support one-screen mirroring support, please list off the game titles. We can have someone run the test ROM (see nesdev thread) on those carts to verify.

dragon2snow commented 8 years ago

Map( 0x9000U, 0x9FFFU, NMT_SWAP_HV ); but This is a bad,because VRC2 unofficial game have four screen support.

koitsu commented 8 years ago

Do you have a list of those games?

koitsu commented 8 years ago

BizHawk (NesHawk) just implemented this change, doing it in a fairly cute way, specifically for mapper 23: they use a variable to differentiate between VRC4e/4f (type=4) and VRC2b (type=2) games. Then for the actual bitmask on writes to the mirroring registers they use (type-1). https://github.com/TASVideos/BizHawk/commit/ef544cd9aba733c20350731f718fdd46edf9fdd1

I thought I'd mention it here since a similar method might be easier to implement in Nestopia.

I didn't look at their code fully/thoroughly to see if they did this for mapper 25 (but would only apply to the game Ganbare Goemon Gaiden: Keita Ougon Kiseru, which is VRC2 but using mapper 25). Reference: http://wiki.nesdev.com/w/index.php/INES_Mapper_025

lidnariq commented 8 years ago

The only blocking issue on this bug is figuring out why Wai Wai World bugs (i.e. fails to draw the computer in the background during attract mode) when using NMT_SWAP_VH.

And the answer is ... Because NMT_SWAP_VH01 and NMT_SWAP_VH mean opposite things for values of 0 and 1! Use NMT_SWAP_HV and everything works fine.

Because Nestopia already emulates the VRC2 and VRC4 separately, there's no need for FCEUX's hack, or BizHawk's separation.

edit: I got these backwards, how embarrassing

dragon2snow commented 8 years ago

qq 20160413065514

joepogo commented 8 years ago

Where is this file located dragon2snow? I cannot seem to find it. Please respond.

lidnariq commented 8 years ago

That's his modified private fork of Nestopia, borrowing the fix from puNES.

There is no need to copypasta like this. Just use NMT_SWAP_HV instead of NMT_SWAP_VH01.

edit: fixed backwards

joepogo commented 8 years ago

Thanks lidnariq, Im trying to figure out where to do this little fix. Im assuming in boards/vrc2?

Map( 0x9000U, 0x9FFFU, NMT_SWAP_VH01 );

to

Map( 0x9000U, 0x9FFFU, NMT_SWAP_VH );

That doesnt work it just changes the screen to black when starting a game...

lidnariq commented 8 years ago

I brainoed on what the original was. Fixed now.

rdanbrook commented 8 years ago

Just so everyone knows, I am paying attention to this issue. I will deal with it soon.

lidnariq commented 8 years ago

joepogo: Change it from VH01 to HV

The problem is that VH01 means "values of 0,1,2,3 are respectively vertical,horizontal,1scA,1scB" but VH means "values of 0,1 are horizontal,vertical":

    NES_POKE_D(Board,Nmt_Vh)
    {
        NST_VERIFY( data <= 0x1 );
        ppu.SetMirroring( (data & 0x1) ? Ppu::NMT_V : Ppu::NMT_H );
    }

This awkward naming convention is what made this bug report go through this long confusing side trip when it's a trivial change.

Perhaps that confusingness should be given its own bug report?

joepogo commented 8 years ago

Sorry for the late reply. Thanks lidnariq! Yea, I changed the VH01 to HV and it works like a charm. Im surprised most fixes for nestopia seem to be very trivial with minimal changes despite the C++ goo spattered everywhere.

Any chance this related to this?:

https://github.com/rdanbrook/nestopia/issues/147

rdanbrook commented 8 years ago

Thanks everyone for doing the analysis and hard work. I just committed the fix.