BizHawk icon indicating copy to clipboard operation
BizHawk copied to clipboard

Replace QuickNES core with QuickerNES

Open SergioMartin86 opened this issue 7 months ago • 14 comments

Update:

This PR now satisfies issue #3848. That is, it integrates the new quickerNES core in replacement of quickNES. This brings performance benefits, additional support for 54 mappers (see below), and support for FourScore (interface not yet added).

Quick test with Arkanoid II shows these changes worked:

Arkanoid II (J) [o1] SHA1:79F9D3DA1904400832546216833978A2261313A5 MD5:BA64EEE98A020760999E43A3650CE0F3

image


This PR updates the quickNES core to support 54 mappers (from the 18 it currently supports), all taken from the latest libretro Quicknes core repository. Here is the full list, new ones marked with an asterisk*.

Mapper  0: nrom
Mapper  1: mmc1
Mapper  2: unrom
Mapper  3: cnrom
Mapper  4: mmc3
Mapper  5: mmc5
Mapper  7: aorom *
Mapper  9: mmc2 
Mapper 10: mmc4 
Mapper 11: color_dreams
Mapper 15: k1029/30P *
Mapper 19: namco106 
Mapper 21: vrc2,vrc4(21) *
Mapper 22: vrc2,vrc4(22) *
Mapper 23: vrc2,vrc4(23) *
Mapper 24: vrc6a
Mapper 25: vrc2,vrc4(25) *
Mapper 26: vrc6b
Mapper 30: Unrom512 *
Mapper 32: Irem_G101 *
Mapper 33: TaitoTC0190 *
Mapper 34: nina1
Mapper 60: NROM-128 *
Mapper 66: gnrom
Mapper 69: fme7 
Mapper 70: 74x161x162x32(70) *
Mapper 71: camerica *
Mapper 73: vrc3 *
Mapper 75: vrc1 *
Mapper 78: mapper_78 *
Mapper 79: nina03,nina06(79) *
Mapper 85: vrc7 *
Mapper 86: mapper_86 *
Mapper 87: mapper_87
Mapper 88: namco34(88) *
Mapper 89: sunsoft2b *
Mapper 93: sunsoft2a *
Mapper 94: Un1rom *
Mapper 97: irem_tam_s1 *
Mapper113: nina03,nina06(113) *
Mapper140: jaleco_jf11 *
Mapper152: 74x161x162x32(152) *
Mapper154: namco34(154) *
Mapper156: dis23c01_daou *
Mapper180: uxrom(inverted) *
Mapper184: sunsoft1 *
Mapper190: magickidgoogoo *
Mapper193: tc112 *
Mapper206: namco34(206) *
Mapper207: taitox1005 *
Mapper232: quattro 
Mapper240: mapper_240 *
Mapper241: mapper_241 *
Mapper246: mapper_246 *

Along with the following changes:

  • I took the chance to re-factor the mess that was the mapper management and file organization. More details in the pull request I opened in the libretro repository.
  • Visual studio obliged me to update the .sln file to VS2022 format. Feel free to bring it back to VS2019 if necessary.
  • I'm uploading the .dll file as I built it; this ought to be taken a look at.

Check if completed:

  • [ ] I have run any relevant test suites
  • [x] I, the committer, have read the [licensing terms for contributors]

SergioMartin86 avatar Jan 07 '24 18:01 SergioMartin86

Interesting note is that a lot of the new files added are GPLv2+ instead of LGPLv2.1+ (that the rest of the core is licensed under). Not really an issue per se (the licensing combo is fine by itself thanks to the +), although this ultimately makes the core GPLv2+ as a whole and no longer LGPLv2.1+.

Also, why are .hpp files used instead of .h files like the rest of quicknes uses?

CasualPokePlayer avatar Jan 08 '24 02:01 CasualPokePlayer

Interesting note is that a lot of the new files added are GPLv2+ instead of LGPLv2.1+ (that the rest of the core is licensed under). Not really an issue per se (the licensing combo is fine by itself thanks to the +), although this ultimately makes the core GPLv2+ as a whole and no longer LGPLv2.1+.

These are the licensing decisions of the authors of the new mappers. I simply kept their license note as-is and made no changes to them.

Also, why are .hpp files used instead of .h files like the rest of quicknes uses?

I use this convention to denote these are exclusively C++ header files. There would be, of course, no problem changing them back to .h if need be.

SergioMartin86 avatar Jan 08 '24 04:01 SergioMartin86

  • Use a submodule rather then checking-in your fork.

Done

  • Please don't globally s/QuickNES/QuickerNES/. You're welcome to change the display name in CoreNames.cs and in readmes.

Done

I could not change the name in CoreNames.cs. If I do that, the core will be undetected. I don't know what the side-effect causing this is. Could you please do the appropriate change(s) for me?

SergioMartin86 avatar Feb 24 '24 07:02 SergioMartin86

Finished the rename for you. The .ico is missing the 32x resolution but I don't think anyone will particularly care. The core name change also breaks config, but again, no-one cares.

Works on my machine so I approve.

Note: after your changes, Quick(er)NES is no longer the default emulator for casual gameplay. If this was intended, it's ok, but you might want to know if this was an unintentional side-effect of the name change.

SergioMartin86 avatar Feb 25 '24 08:02 SergioMartin86

It still is, it's only existing configs that will be broken, as I said.

YoshiRulz avatar Feb 25 '24 20:02 YoshiRulz

It still is, it's only existing configs that will be broken, as I said.

I see. Yes, I re-ran from a clean slate and it works as intended.

Now there's another issue I observed. In TAStudio, all the frames are deemed lag frames (painted in red)

image

is this related to the renaming or because I removed the trace callback routine in Qnes' core? I can re-implement it, if that's the case.

SergioMartin86 avatar Feb 26 '24 05:02 SergioMartin86

@SergioMartin86 In the existing code on master, lag frame counts are supported by qn_get_joypad_read_count. Does that still work?

nattthebear avatar Feb 26 '24 16:02 nattthebear

@SergioMartin86 In the existing code on master, lag frame counts are supported by qn_get_joypad_read_count. Does that still work?

ah, no, I removed the joy read count for performance. This counter was in the very heart of the emulation core and accounted for some % of the running time. I will put it back conditionally for BizHawk. I don't need it for my botting purposes, but it's very useful when copying the bot results back into BK.

SergioMartin86 avatar Feb 26 '24 17:02 SergioMartin86

Thanks. Yes, I think we'll want the lag counter here; this core should be fast, and is much faster than NesHawk, and that's great. But it also needs to support the needs of bot-assisted TASes, which sometimes work with the lag counter.

nattthebear avatar Feb 26 '24 17:02 nattthebear

Maybe in the future lag info could be turned into just a flag inside the core to avoid going back and forth with callbacks? Or am i not getting why trace callback is involved?

vadosnaprimer avatar Feb 26 '24 19:02 vadosnaprimer

Maybe in the future lag info could be turned into just a flag inside the core to avoid going back and forth with callbacks?

this is exactly what I just did. Lag frame detection and cpu tracing are supported again via defines

image

Edit: If you meant having an option to enable/disable it on runtime, this won't work. Just having a simple 'if' statement in the register read routine will affect performance in a non-negligible way. The only way is to make it a compilation-time decision. I will disable it for my personal project, for example.

SergioMartin86 avatar Feb 26 '24 19:02 SergioMartin86

I have no further developments to do in this PR, everything seems to work just fine now. You'll have to re-approve it though

SergioMartin86 avatar Feb 26 '24 19:02 SergioMartin86

I didn't mean compile flags, just a variable in the code, for the current frame (lag or not). Still too slow?

vadosnaprimer avatar Feb 26 '24 19:02 vadosnaprimer

I didn't mean compile flags, just a variable in the code, for the current frame (lag or not). Still too slow?

yes... even a single if statement affects performance in this part of the core.

SergioMartin86 avatar Feb 26 '24 19:02 SergioMartin86

I rewrote the gamepad code, so you should be good to add FourScore support now. Just fill in the TODOs in those 3 files. I'm not sure what you meant by "Fourscore (port 2, it differs in signature)", but if you meant the button names and/or bit order is different, that should be handled. For the virtual pads, you may be able to re-use the helper used for NesHawk earlier in the file.

YoshiRulz avatar Mar 19 '24 22:03 YoshiRulz

I rewrote the gamepad code, so you should be good to add FourScore support now. Just fill in the TODOs in those 3 files.

Are these TODOs for me to fill in? Because I'm not quite understanding what needs to be written to satisfy them.

I'm not sure what you meant by "Fourscore (port 2, it differs in signature)", but if you meant the button names and/or bit order is different, that should be handled.

Cool, yes, that's what I meant.

For the virtual pads, you may be able to re-use the helper used for NesHawk earlier in the file.

Again, if this refers to me, I also don't know what this means.

SergioMartin86 avatar Mar 20 '24 11:03 SergioMartin86

Create a FourScoreButtons like GamepadButtons (outer index is obviously player no.; inner index is order in keybinds menu, first in each pair is the button name used in keybinds, Lua, etc., second in each pair is arbitrary but I expect a bitmask like the gamepads will be suitable), then create a local method PackFourScoreButtonsFor under PackGamepadButtonsFor where you implement the bitfield logic. If it differs between the ports, you'll need to make two, or add a bool param. For the virtual pads: I was mistaken, there's no FourScore helper, rather it returns 2 gamepads. If that's not suitable and you can't figure out what you need to do, leave it and I'll have another look.

YoshiRulz avatar Mar 20 '24 11:03 YoshiRulz

Create a FourScoreButtons like GamepadButtons (outer index is obviously player no.; inner index is order in keybinds menu, first in each pair is the button name used in keybinds, Lua, etc., second in each pair is arbitrary but I expect a bitmask like the gamepads will be suitable), then create a local method PackFourScoreButtonsFor under PackGamepadButtonsFor where you implement the bitfield logic. If it differs between the ports, you'll need to make two, or add a bool param. For the virtual pads: I was mistaken, there's no FourScore helper, rather it returns 2 gamepads. If that's not suitable and you can't figure out what you need to do, leave it and I'll have another look.

Thanks for the explanation. I had jumped the gun too soon and the changes were much easier to make than I thought. I used a controller test rom and every button works ok. Here's a capture of R.C. Pro-Am II

image

SergioMartin86 avatar Mar 20 '24 17:03 SergioMartin86

Resynced to master HEAD

SergioMartin86 avatar Mar 21 '24 15:03 SergioMartin86