01858: gijoe: Zooming glitches and a wrong backdrop

ID	Category [?]	Severity [?]	Reproducibility	Date Submitted	Last Update
01858	Graphics	Minor	Always	Jun 1, 2008, 12:55	13 days ago

Tester	Kekule	View Status	Public	Platform
Assigned To	hap	Resolution	Fixed	OS
Status [?]	Resolved			Driver	konami/gijoe.cpp
Version	0.125u3	Fixed in Version	0.280GIT	Build
		Fixed in Git Commit	8a34e47	Github Pull Request #

Summary	01858: gijoe: Zooming glitches and a wrong backdrop
Description	After the "elevator fixing", GI Joe shows some glitches in the elevator (3rd) level, while grates zoom and scroll down. Once you reach the end level boss, the backdrop promptly changes to an obvious wrong one (probably because when the zooming ends, it changes in a common tiles background graphics, but it chooses the wrong one), with robot octopuses popping up from...uh from nowhere?
Steps To Reproduce	Go to the 3rd level; reach the 3rd level's boss.
Additional Information
Github Commit

Flags
Regression Version	0.118u2
Affected Sets / Systems	gijoe

Attached Files	gijoe0000.png (15,374 bytes) Jun 1, 2008, 12:55
	gijoe0001.png (13,867 bytes) Jun 1, 2008, 12:55
	gijoe.zip (50,577 bytes) Oct 29, 2018, 10:32 Uploaded by MetalGod
	mame 0.110.png (19,022 bytes) Oct 29, 2018, 11:03 Uploaded by MetalGod

No.01172 haynor666 Tester Jun 1, 2008, 15:23	Related to bug 0484.
No.15645 MetalGod Senior Tester Oct 29, 2018, 10:31 edited on: Oct 29, 2018, 10:34	Still happening in mame 0.202 The only glitch I see is the boss floor. The boss floor is a different backdrop not seen in the previously uploaded images. It doesn't appear in MAME. The rest of level 3 seems ok in current version. I've uploaded savestates for the beginning of level 3, and right before the boss. Here's a video of the original arcade pcb. Level 3 is in 10:28 . Boss is in 12:30
No.15646 MetalGod Senior Tester Oct 29, 2018, 10:50 edited on: Oct 29, 2018, 17:20	Runs perfect in mame 0.115 backwards, the floor backdrop is correct in the boss. I've uploaded a image of it Regression is 0.118u2, right when the related elevator issue (00484) was solved
No.17119 Haze Senior Tester Oct 22, 2019, 14:58	it was broken intentionally as the code to support it was one of the biggest abuses of code in MAME (writing directly to the private tilemap cache rather than properly emulating it)
No.23576 hap Developer 15 days ago edited on: 15 days ago	The hacky rendering still works, it just needed 17 years worth of maintenance updates. I think the reason Haze disabled it was because he wasn't able to fix it, not because it's hacky. If it was just that, the sane thing would be to fix it properly 17 years ago and remove the old method afterwards. It's unfortunate that the game was left in a broken state for so long. I've audited the code and I don't think it is unsafe code or will crash, but yes, rendering directly to the tilemap is not the right method. Fixed in 2 parts: 1: https://github.com/mamedev/mame/commit/8d1c808aa144dde53356328e34fc9e8282ffedb5 2: https://github.com/mamedev/mame/commit/8a34e47a1cb74d508d45218164f707a4f9f9ffa1 The 17yr old commit where it got disabled: https://github.com/mamedev/mame/commit/4afa8414cab718b49bff2f90fa50bc76d2e5ad87 fix try after: https://github.com/mamedev/mame/commit/4a1e7227782fd349a393cd0a6c8061c9a0eb4eed The commit that caused the crash seen in https://mametesters.org/view.php?id=484 ? I don't know, probably an update to tilemap or drawgfx.
No.23578 Haze Senior Tester 14 days ago	Well it's more it was such a gross abuse of the MAME systems I didn't want to fix it, as it might have encouraged similar gross abuse. I'm not even sure fixing the existing code would have passed the other devs even back then. I'm a little surprised nobody rewrote it to work properly in that time, because clearly the logic is somewhat correct, but that implementation is just abhorrent.
No.23580 hap Developer 14 days ago	In hindsight, if you'd spent an hour or 2 understanding the hacky code and making it work, then you or next person that comes along for a refactor later on, would have a working reference they can convert to a better implementation. Why it wasn't fixed the last 17 years: probably people waiting for the Konami rewrite(tm)? And after a few years, the issue was forgotten about.
No.23583 TheCoolPup Tester 14 days ago	i noticed the same issue happens midway the 2nd stage as well
No.23584 Haze Senior Tester 13 days ago	We'll see if Vas kicks off about it anyway, I'm 99% sure if I attempted to submit that changeset via a PR I'd be slaughtered for it.
No.23585 hap Developer 13 days ago edited on: 13 days ago	It's not a newly added hack, and your #if 0 around it didn't make it any better - half of the hack was still enabled. What should we do to other hacky code in MAME, add an #if 0 and pray someone else will write accurate emulation? BTW that large group of gijoe game fans you mentioned that refuse to update MAME specifically due to this bug, which you complained about on the shoutbox on their behalf (though you were probably exaggerating). Did they find another excuse to not update MAME yet?
No.23586 Haze Senior Tester 13 days ago edited on: 13 days ago	Who knows. It's a popular game with some crowds, and the problem is with popular games, if fans of it see it's broken, they assume the rest of MAME is going to be full of bugs compared to the old versions too. That code is bad though, it was left as reference, but writing into what should be a private structure (once it became apparent that's what it was doing) isn't something I'd have expected devs to accept being re-enabled even back then, when standards were less strict. If Vas doesn't kick off about it, great, because yes, the game being broken does make MAME look bad, but Vas is even more focused on 'bad code' than I am, so I fear he'll just nuke it when it comes to release time.
No.23587 hap Developer 13 days ago edited on: 13 days ago	Yes publically accessible tilemap data pointers can be read/written to. It's already writing to the tilemap data before the part you disabled, this part was not inside the #if 0, see: tmap = m_tilemap[page]; bitmap_ind8 &xprmap = tmap->flagsmap(); xprdata = tmap->tile_flags(); (...) tmap->mark_all_dirty(); tmap->pixmap(); // dummy call to reset tile_dirty_map xprmap.fill(0); // reset pixel transparency_bitmap; memset(xprdata, TILEMAP_PIXEL_LAYER0, 0x800); // reset tile transparency_data; Hence my belief that you disabled the pixmap part was because you couldn't get it working, not because it's hacky. Above code is already hacky enough.
No.23588 Haze Senior Tester 13 days ago edited on: 13 days ago	Well I could have put the #if 0 somewhere else, that was to stop it crashing outright. but the code itself was so bad I had no interest in even trying to make it work again, it shouldn't have been there in the first place, just wasn't caught when it went in. it struck me as fragile code playing with internal structures it had no business touching (that could change, and likely did change, leading to the breakage in the first place) no attempt was made to fix it beyond disabling what was causing the crash. you're probably right about the ongoing Konami rewrite though, that's been over a decade of nobody wanting to touch the Konami drivers until now due to not wanting to interfere with something they can't see - that's being handled incredibly badly.