08520: dkong3, dkong3hs, dkong3j: Stanley death sound only plays partially

ID	Category [?]	Severity [?]	Reproducibility	Date Submitted	Last Update
08520	Sound	Minor	Always	Nov 25, 2022, 06:42	Jan 16, 2023, 21:00

Tester	kmg	View Status	Public	Platform	MAME (Self-compiled)
Assigned To	hap	Resolution	Fixed	OS	MacOS X
Status [?]	Resolved			Driver	nintendo/dkong.cpp
Version	0.249	Fixed in Version	0.251	Build	64-bit
		Fixed in Git Commit	c616219	Github Pull Request #

Summary	08520: dkong3, dkong3hs, dkong3j: Stanley death sound only plays partially
Description	See summary.
Steps To Reproduce	Start game, get killed.
Additional Information	Looks like the regression is my commit: https://github.com/mamedev/mame/commit/d8fd71415204a2c91bdebf199c2992a5fc6c9133 That commit was mostly removing unused cruft from the NES APU. It does however remove the ability to read back values written to the APU's registers which seems to be causing the problem. On the NES reading anything but the status register should return open bus (we don't do this yet).
Github Commit

Flags
Regression Version	0.242
Affected Sets / Systems	dkong3, dkong3hs, dkong3j

Attached Files	dkong3.png (191,097 bytes) Dec 3, 2022, 21:19 Uploaded by hap
Attached Files	punchout_rev1.png (255,766 bytes) Dec 3, 2022, 21:20 Uploaded by hap

No.20861 Augusto Tester Nov 25, 2022, 18:25	Tested Ubuntu 20.04 SDLMAME 0.249 64 bits. Following the bug instructions. Here not any issues. The player lose sound is played correctly. You can test the Linux build ?
No.20862 Tafoid Administrator Nov 25, 2022, 20:02	I can confirm with current Git and 0.249 in Windows. Best way to duplicate is simply jump up to the third (top) tier and wait to get hit. Happens every time.
No.20866 Augusto Tester Nov 26, 2022, 14:34	I move the player character to top and when lose the sound is played completely. Not happening in Linux version ? Have moments is good have the windows build only for test using WINE. Windows build run correctly in WINE current version. Kmg and Tafoid are very much charitable ... I must post an video capture here. I will to do it in next days.
No.20871 Augusto Tester Nov 29, 2022, 13:40	Here an video capture. Emulated sound is correct ?
No.20872 mrchrisster Tester Nov 29, 2022, 21:17 edited on: Nov 29, 2022, 21:24	@Augusto Your video plays the incomplete death sound. Check these two videos for comparison: https://www.youtube.com/shorts/WOlEXLiAk4s https://youtube.com/clip/Ugkx3JFM8oMOA0suqUvWzCHjA5GJ9elaGo5_
No.20873 Augusto Tester Nov 29, 2022, 22:31	Thanks adding the link.
No.20874 hap Developer Nov 30, 2022, 19:33	Nothing weird in the schematics (in theory, 0x4xxx could have been shared with RAM) RAM at !A14 ROM at A15 Maybe the one used in NES is a bit different from the standalone RP2A03 chip.
No.20875 kmg Senior Tester Nov 30, 2022, 20:50 edited on: Nov 30, 2022, 20:55	I looked into this a tiny bit more last weekend after posting the initial bug report. Sorry, the report is misleading. At the time I thought dkong3 was reading the APU regs just from some quick and dirty printfs in nes_apu.cpp. Actually it's indirect STAs that are causing nes_apu::read() to be called. I didn't understand why. I don't think dkong3 ever actually tries to read the APU other than the one readable address $4015. However, one place the indirect reads are happening is on CPU ":rp2a03a", at EAAE: sta $4002, x. I also couldn't figure out why any of this mattered and could only confirm that putting the stored reg array back into the apu_t struct and restoring the two lines in read() and write() fixed the damage. That's as far as I got with any of it and haven't had a chance to dig back in.
No.20876 hap Developer Nov 30, 2022, 21:11	rp2a03b does: ldx $4002 inx stx $4002 and does the same for $4006 and $400a
No.20877 kmg Senior Tester Nov 30, 2022, 21:30	Ok, that's interesting. Maybe dkong3 does want to be able to read back those timers. If I recall correctly the death sound is played on both APUs (both?) pulse channels, so $4002 and $4006 would be relevant.
No.20878 kmg Senior Tester Nov 30, 2022, 21:36	FWIW, I also looked for something like shared RAM in the dkong3 schematics. Another possibility (in addition to being completely off course :) ) is that some of the APU registers WERE readable in the RP2A03 letterless revision and it's the same on early Famicoms. I found nothing corroborating this hypothesis, but letterless is definitely not as well understood.
No.20886 hap Developer Dec 1, 2022, 12:52	According to this thing, the registers are r/w http://www.qmtpro.com/~nes/chipimages/visual2a03/
No.20898 kmg Senior Tester Dec 3, 2022, 20:31	hap, sorry, I've already been wrong several times here, but if you'll humor me again :) Yeah, now that I check both the 2A03s have that same code you posted, so they definitely need to read back those addresses. Still, which schematic did you look at? I just looked into this again, but the common dkong3 schematic is terribly illegible. But it should be that dkong3 and the punchout related boards all have fairly similar setups around the 2A03s, which helps with reading the dkong3 one. In this case the address decoding, as it were, is just A15 directly connected to both RAM and ROM's output enable pins. So it should be that RAM is mirrored across the lower half of memory and ROM across the upper half. It seems the solution really is just 0x4xxx being shared with RAM. AFAICT, all these games mostly avoid writing zero page RAM that is mirrored where the APU regs overlap at 0x4000, implying the coders know that APU writes are going to overwrite RAM there. All except dkong3 that is, which depends on incrementing $4002/6/a.
No.20899 hap Developer Dec 3, 2022, 20:47	dkong 3 schematics (the scan that I know of) is really blurry and I thought it's the same as Punch Out but nope it's not. you can see the number "19", which is A15. !A15 = RAM A15 = ROM MAME's 2A03 doesn't support any bus conflicts right now though. Punch Out schematics is like I said: !A14 = RAM A15 = ROM
No.20900 kmg Senior Tester Dec 3, 2022, 21:05 edited on: Dec 3, 2022, 21:08	Are there different revisions of the Punch Out schematics? What I see is both dkong3 and punchout having pin 19 connected as you've just described. There is however an Arm Wrestling schematic that looks like A15 = ROM and !A14 = RAM, though. EDIT: Ok, I see, there really are two revisions of the Punch Out schematic.
No.20901 hap Developer Dec 3, 2022, 21:18	The punchout schematics I downloaded (not sure I can link to full manual here), is revision 1. I'll attach pics.
No.20909 hap Developer Dec 4, 2022, 14:05	That was pretty annoying to fix, it would have been much easier if the driver memory map had priority over the 2A03 internal memory map instead of the other way around. part1: https://github.com/mamedev/mame/commit/a8c8154b43fc8442b7b616fbaac3f0b18d59583e part2: https://github.com/mamedev/mame/commit/c6162199db4b6c36d2ce982e25e2d097d6646007
No.20972 kmg Senior Tester Dec 21, 2022, 19:13 edited on: Dec 21, 2022, 19:20	I did some further QA testing now that this is fixed. It's worth noting that the Creepy inchworm sound (triangle channel on rp2a03a) was clipped too to the point it sounded more like a percussive sound. The jumping down a platform sound was also up an octave or two. Now that Creepy's sound is better I also investigated a high pitch noise he makes. This occurred pre-regression and again post-fix. Looks like it's a BTANB and can be heard after Creepy gets shot the 2nd time after the linked timestamp in this video:
No.20976 Augusto Tester Dec 22, 2022, 07:17	kmg doing the good deed. Have an nice week.
No.21015 mrchrisster Tester Jan 15, 2023, 05:23	Just got 251 but this fix is still not in Mainline mame hey?
No.21016 hap Developer Jan 15, 2023, 12:48	I'm not sure why you say that. The bug was pretty subtle, maybe that's why? The fix here was: player gets hit and dies, old version: fall sound plays for a short duration fixed in 0.251: fall sound is ok
No.21019 mrchrisster Tester Jan 16, 2023, 21:00 edited on: Jan 16, 2023, 21:01	I compiled my own mame with the fixes where fall sound is fine but installing mame 251 fall sound was still broken. It's groovymame so let me check with mainline mame if fall sound is fixed now