libretro / prosystem-libretro

Port of ProSystem to the libretro API.
GNU General Public License v2.0
14 stars 40 forks source link

@twinpaphex PS3 port of Prosystem not referencing Database.c values when loading games #11

Closed underball closed 8 years ago

underball commented 8 years ago

Although the Prosystem flags, region, pokey and type values are being detected via MD5 and passed in the Windows and Android ports of Retroarch, this is not happening in the PS3 1.3.6 beta. This causes any games with non-standard Type, Flags, Pokey, or Region values to either crash Retroarch, or not function properly.

Games to test this include:

Commando (Atari - Doesn't load, crashes PS3 console and requires manual restart) Double Dragon (Activision - game doesn't load, however, you can return to the RA menu) Kung Fu Master (Activision - graphics are garbled, game is unplayable)

All these games are working on the Windows and Android ports of Retroarch (and every other port of Prosystem in existence)

Roms of these games can be found here for testing purposes: http://www.atariage.com/system_items...ItemTypeID=ROM

Ezio-PS commented 8 years ago

There could be a little of code not endian-safe which doesn't allow to the core to work such good as happens in other platforms. I tried to debug the core on ps3 while loading Double Dragon, here is the log. http://pastebin.com/sPujdwTZ Anyway i don't see info too relevant, no idea what line 57 means. RetroArch [INFO] :: Did not find a valid content patch.

underball commented 8 years ago

Here's a dump from the WORKING Windows port of REtroarch. See the line below setting the system environment: (last line). That line never appears in the PS3 log. That's where it checks the internal database for the file options.

Skipping SRAM load.. Uersion of libretro API: Compiled against API: 1 Set audio input rate to: Uideo 8 960x720 Found GL context: wgl Detecting screen resolution IWGLA: wg1SwapInterval(1) EGLI]: Uendor: ATI Technologies Inc., Renderer: AMD Radeon HD 1 47960.00 Hz. 1920x1080. EGLI]: Uersion: EGLI]: ATI card 4.2.11631 Compatibility Profile Context. detected, skipping check for GL_RGB565 suppor A I4* C:\retroarch\retroarch_debua.exe WWII 1 2 RetroArch (INFO] :: Loading dynamic libretro core from: "C:\retroarch\cores\pros ystem_libretro.d11" RetroArch (INFO] :: Overrides: no core-specific overrides found at C:\retroarch\ config\ProSystem\ProSystem.cfg RetroArch (INFO] :: Overrides: no game-specific overrides found at C:\retroarch\ config\ProSystem\Double Dragon.cfg RetroArch (INFO] :: Remaps: core name: ProSystem RetroArch (INFO] :: Remaps: game name: Double Dragon RetroArch (INFO] :: Remaps: remap directory: C:\retroarch\config\remap RetroArch (INFO] :: Remaps: no game-specific remap found at C:\retroarch\config\ remap\ProSystem\Double Dragon.rmp RetroArch (INFO] :: Remaps: no core-specific remap found at C:\retroarch\config\ renap\ProSystem\ProSystem.rmp RetroArch (INFO] :: Environ GET_LOG_INTERFACE. RetroArch (INFO] :: Environ PERFORMANCE_LEUEL: 5. RetroArch (INFO] :: Loading content file: C:\retroarch\ROMS\a7800\Double Dragon. an. RetroArch (INFO] Did not find a valid content patch. RetroArch (INFO] CRC32: Ox16e2fc12 RetroArch (INFO] Environ SETJ'IXELJlORMAT: XRGB8888. RetroArch (INFO] Environ SET_INPUTJESCRIPTORS: RetroArch (INFO] RetroPad, User 1, Button "B (bottom)" => "1" RetroArch (INFO] RetroPad, User 1, Button "Select" => "Console Select" RetroArch (INFO] RetroPad, User 1, Button "Start" => "Console Pause" RetroArch (INFO] RetroPad, User 1, Button "D-Pad Up" => "Up" RetroArch (INFO] RetroPad, User 1, Button "D-Pad Down" => "Down" RetroArch (INFO] RetroPad, User 1, Button "D-Pad Left" => "Left" RetroArch (INFO] RetroPad, User 1, Button "D-Pad Right" => "Right" RetroArch (INFO] RetroPad, User 1, Button "A (right)" => "2" RetroArch (INFO] RetroPad, User 1, Button "X (up)" => "Console Reset" RetroArch (INFO] RetroPad, User 1, Button "L" => "Left Difficulty" RetroArch (INFO] RetroPad, User 1, Button "R" => "Right Difficulty" RetroArch (INFO] :: RetroPad, User 2, Button "B (bottom)" => "1" RetroArch (INFO] :: RetroPad, 2, User Button "D-Pad Up" => "Up" RetroArch (INFO] :: RetroPad, User 2, Button "D-Pad Down" => "Down" RetroArch (INFO] :: RetroPad, User 2, Button "D-Pad Left" => "Left" RetroArch (INFO] :: RetroPad, User 2, Button "A (right)" => "2" RetroArch (INFO] :: Environ SYSTEM_DIRECTORY: "C:\retroarch\system". Found entry in internal database: Double Dragon (543484c00ba233736bcaba2da20eeea 9]

Alcaro commented 8 years ago

Anyway i don't see info too relevant, no idea what line 57 means. RetroArch [INFO] :: Did not find a valid content patch.

Softpatching. Completely normal and harmless.

Also forced it to print the hash for entries it can't find, so we can figure out if it's hashing something bad or if the bug is in the database.

Ezio-PS commented 8 years ago

Okay, i'm going to compile just now, so we can do an immediate compare.

I just did 3 tests: 1) i loaded Karateka, it doesn't boot and RA crashes. Log here http://pastebin.com/87JiEvyk 2) i loaded Rampage, it boots but i get only a black screen. Log here http://pastebin.com/44Hz4Zsc 3) i loaded Mario Brothers, it doesn't boot and RA crashes. Log here http://pastebin.com/7pwrhdLE

I noticed another log coming from ps3 that seems interesting. http://pastebin.com/HzfBfTNm Here you can see at the second and at third content laoded (such as 2 times Rampage) I got Did not find entry in internal database: [564480c05b5407fe6e29a650256cf1d0] on line 9 and 15.

That hash doesn't seem to be into the database though. I'm loading unzipped contents.

I tried Fight Night, it boots then the screen becomes grey. Log http://pastebin.com/UwKQ7td7 And http://pastebin.com/g2upVzic

Well, it seems it's not included into the database too.

I'll try to include them into the database to see what happens, tomorrow.

Ezio-PS commented 8 years ago

@Alcaro The issue seems to be related to a different recognize of the hash between ps3 and the other platforms (something bad in the hashing??). The following two examples does it really clear:

1) Rampage (1989)(!) on Windows

Found entry in internal database: Rampage [ac03806cef2558fc795a7d5d8dba7bc0]

on PS3

Did not find entry in internal database: [564480c05b5407fe6e29a650256cf1d0]

2) Fight Night

on Windows

Found entry in internal database: Fight Night [07dbbfe612a0a28e283c01545e59f25e]

on PS3

Did not find entry in internal database: [57b5e213987cd769d51211cf165c1d2b]

Then i inserted this two hash values in the database, compiled the core for ps3 and they're working fine now. (before black screen)

Should we insert all the new entries for the ps3 platform (???), or would it better be to fix the hashing for ps3 (tbh no idea what to do) ?

Alcaro commented 8 years ago

uint8_t ptr = (uint8_t)buffer3 + temp; hash_Transform(buffer1, (uint32_t*)buffer3);

Yeah, that one's bad news. Typecasts are not endian safe, and PS3 is an unusual endian.

My recommendation: Replace Hash.c with an actually working MD5 function, then regenerate all hash entries. This will also break savestates, but I believe that's worth it; homebrew or broken crypto is always a bad idea.

Duplicating the database is a bad idea; that file is already big enough, we don't want to double that. Nor do we want to deal with the bugs/confusion if a ROM ends up without its corresponding big-endian variant. Or force people to get a PS3 to add anything to that database.

underball commented 8 years ago

I think a big part of the issue is that the Prosystem.DAT file was "baked in" when it was ported to libretro. This is, in my opinion, a mistake. I understand that the libretro team wants to eliminate as many external dependency files as possible, but in this particular case - the Prosystem.DAT file is constantly being updated. The Atari 7800 homebrew game development and hacking scene is more active than any other retro gaming scene. There are dozens of Homebrew games and hacks that require new entries to this database file, both existing and currently WIP development. By not having this file as an external, user-editable resource, you're limiting the development scene for the 7800.

/gripe session off

Ezio-PS commented 8 years ago

Definitively fixed by this commit https://github.com/libretro/prosystem-libretro/commit/5430fbd8026544eb817398268d29f8806257a5a2