Merge several fixes from PCSX Redux and adjust delay for SetLocPending. (#221)

There's a game, PoPoLoCrois Monogatari II, that unfortunately locks up
during the intro screen.
I should have known that code was wrong as Mednafen did not have anything
like that in their code either, hence the confusion.

Their fix however still don't include the Driver fix so the game would still
crash if we don't have the "+ Seektime".
To be honest, i'm not sure why the PCSX Reloaded team did it this way...

I noticed that the fastforward and fastbackward code was pretty much unused.
Looked at Mednafen and all they do is just adjust the cursector
and make sure that fastword & backword trigger the AUTO_REPORT code
so i did the latter.

Co-authored-by: Nicolas Noble <nicolasnoble@users.noreply.github.com>

diff --git a/libpcsxcore/cdriso.c b/libpcsxcore/cdriso.c
index 9ef594c..5aad225 100644 (file)
--- a/libpcsxcore/cdriso.c
+++ b/libpcsxcore/cdriso.c
@@ -1480,12 +1480,12 @@ static void DecodeRawSubData(void) {
 // read track
 // time: byte 0 - minute; byte 1 - second; byte 2 - frame
 // uses bcd format
-static long CALLBACK ISOreadTrack(unsigned char *time) {
+static boolean CALLBACK ISOreadTrack(unsigned char *time) {
        int sector = MSF2SECT(btoi(time[0]), btoi(time[1]), btoi(time[2]));
        long ret;
 
        if (cdHandle == NULL) {
-               return -1;
+               return 0;
        }
 
        if (pregapOffset) {
@@ -1499,7 +1499,7 @@ static long CALLBACK ISOreadTrack(unsigned char *time) {
 
        ret = cdimg_read_func(cdHandle, 0, cdbuffer, sector);
        if (ret < 0)
-               return -1;
+               return 0;
 
        if (subHandle != NULL) {
                fseek(subHandle, sector * SUB_FRAMESIZE, SEEK_SET);
@@ -1508,7 +1508,7 @@ static long CALLBACK ISOreadTrack(unsigned char *time) {
                if (subChanRaw) DecodeRawSubData();
        }
 
-       return 0;
+       return 1;
 }
 
 // plays cdda audio

diff --git a/libpcsxcore/cdrom.c b/libpcsxcore/cdrom.c
index 47e4037..24fd9c9 100644 (file)
--- a/libpcsxcore/cdrom.c
+++ b/libpcsxcore/cdrom.c
@@ -383,7 +383,7 @@ static void ReadTrack(const u8 *time) {
 
        CDR_LOG("ReadTrack *** %02x:%02x:%02x\n", tmp[0], tmp[1], tmp[2]);
 
-       cdr.RErr = CDR_readTrack(tmp);
+       cdr.NoErr = CDR_readTrack(tmp);
        memcpy(cdr.Prev, tmp, 3);
 
        if (CheckSBI(time))
@@ -450,7 +450,7 @@ static void cdrPlayInterrupt_Autopause()
 
                StopCdda();
        }
-       else if (cdr.Mode & MODE_REPORT) {
+       else if (((cdr.Mode & MODE_REPORT) || cdr.FastForward || cdr.FastBackward)) {
                CDR_readCDDA(cdr.SetSectorPlay[0], cdr.SetSectorPlay[1], cdr.SetSectorPlay[2], (u8 *)read_buf);
                cdr.Result[0] = cdr.StatP;
                cdr.Result[1] = cdr.subq.Track;
@@ -612,6 +612,10 @@ void cdrInterrupt() {
                                // XXX: wrong, should seek instead..
                                cdr.Seeked = SEEK_DONE;
                        }
+                       
+                       cdr.FastBackward = 0;
+                       cdr.FastForward = 0;
+
                        if (cdr.SetlocPending) {
                                memcpy(cdr.SetSectorPlay, cdr.SetSector, 4);
                                cdr.SetlocPending = 0;
@@ -676,9 +680,7 @@ void cdrInterrupt() {
                        cdr.Stat = Complete;
 
                        // GameShark CD Player: Calls 2x + Play 2x
-                       if( cdr.FastForward == 0 ) cdr.FastForward = 2;
-                       else cdr.FastForward++;
-
+                       cdr.FastForward = 1;
                        cdr.FastBackward = 0;
                        break;
 
@@ -686,9 +688,7 @@ void cdrInterrupt() {
                        cdr.Stat = Complete;
 
                        // GameShark CD Player: Calls 2x + Play 2x
-                       if( cdr.FastBackward == 0 ) cdr.FastBackward = 2;
-                       else cdr.FastBackward++;
-
+                       cdr.FastBackward = 1;
                        cdr.FastForward = 0;
                        break;
 
@@ -955,7 +955,22 @@ void cdrInterrupt() {
                case CdlReadS:
                        if (cdr.SetlocPending) {
                                seekTime = abs(msf2sec(cdr.SetSectorPlay) - msf2sec(cdr.SetSector)) * (cdReadTime / 200);
-                               if(seekTime > 1000000) seekTime = 1000000;
+                               /*
+                               * Gameblabla :
+                               * It was originally set to 1000000 for Driver, however it is not high enough for Worms Pinball
+                               * and was unreliable for that game.
+                               * I also tested it against Mednafen and Driver's titlescreen music starts 25 frames later, not immediatly.
+                               * 
+                               * Obviously, this isn't perfect but right now, it should be a bit better.
+                               * Games to test this against if you change that setting :
+                               * - Driver (titlescreen music delay and retry mission)
+                               * - Worms Pinball (Will either not boot or crash in the memory card screen)
+                               * - Viewpoint (short pauses if the delay in the ingame music is too long)
+                               * 
+                               * It seems that 3386880 * 5 is too much for Driver's titlescreen and it starts skipping.
+                               * However, 1000000 is not enough for Worms Pinball to reliably boot.
+                               */
+                               if(seekTime > 3386880 * 2) seekTime = 3386880 * 2;
                                memcpy(cdr.SetSectorPlay, cdr.SetSector, 4);
                                cdr.SetlocPending = 0;
                                cdr.m_locationChanged = TRUE;
@@ -987,22 +1002,29 @@ void cdrInterrupt() {
                        - fixes cutscenes
                        C-12 - Final Resistance - doesn't like seek
                        */
+                       
+                       /*      
+                               By nicolasnoble from PCSX Redux :
+                               "It LOOKS like this logic is wrong, therefore disabling it with `&& false` for now.
+                               For "PoPoLoCrois Monogatari II", the game logic will soft lock and will never issue GetLocP to detect
+                               the end of its XA streams, as it seems to assume ReadS will not return a status byte with the SEEK
+                               flag set. I think the reasonning is that since it's invalid to call GetLocP while seeking, the game
+                               tries to protect itself against errors by preventing from issuing a GetLocP while it knows the
+                               last status was "seek". But this makes the logic just softlock as it'll never get a notification
+                               about the fact the drive is done seeking and the read actually started.
+
+                               In other words, this state machine here is probably wrong in assuming the response to ReadS/ReadN is
+                               done right away. It's rather when it's done seeking, and the read has actually started. This probably
+                               requires a bit more work to make sure seek delays are processed properly.
+                               Checked with a few games, this seems to work fine."
+                               
+                               Gameblabla additional notes :
+                               This still needs the "+ seekTime" that PCSX Redux doesn't have for the Driver "retry" mission error.
+                       */
+                       cdr.StatP |= STATUS_READ;
+                       cdr.StatP &= ~STATUS_SEEK;
 
-                       if (cdr.Seeked != SEEK_DONE) {
-                               cdr.StatP |= STATUS_SEEK;
-                               cdr.StatP &= ~STATUS_READ;
-
-                               // Crusaders of Might and Magic - use short time
-                               // - fix cutscene speech (startup)
-
-                               // ??? - use more accurate seek time later
-                               CDREAD_INT(((cdr.Mode & 0x80) ? (cdReadTime / 2) : cdReadTime * 1) + seekTime);
-                       } else {
-                               cdr.StatP |= STATUS_READ;
-                               cdr.StatP &= ~STATUS_SEEK;
-
-                               CDREAD_INT((cdr.Mode & 0x80) ? (cdReadTime / 2) : cdReadTime * 1);
-                       }
+                       CDREAD_INT(((cdr.Mode & 0x80) ? (cdReadTime) : cdReadTime * 2) + seekTime);
 
                        cdr.Result[0] = cdr.StatP;
                        start_rotating = 1;
@@ -1126,9 +1148,9 @@ void cdrReadInterrupt() {
 
        buf = CDR_getBuffer();
        if (buf == NULL)
-               cdr.RErr = -1;
+               cdr.NoErr = 0;
 
-       if (cdr.RErr == -1) {
+       if (!cdr.NoErr) {
                CDR_LOG_I("cdrReadInterrupt() Log: err\n");
                memset(cdr.Transfer, 0, DATA_SIZE);
                cdr.Stat = DiskError;
@@ -1536,9 +1558,7 @@ void cdrReset() {
        cdr.DriveState = DRIVESTATE_STANDBY;
        cdr.StatP = STATUS_ROTATING;
        pTransfer = cdr.Transfer;
-       cdr.SetlocPending = 0;
-       cdr.m_locationChanged = FALSE;
-
+       
        // BIOS player - default values
        cdr.AttenuatorLeftToLeft = 0x80;
        cdr.AttenuatorLeftToRight = 0x00;

diff --git a/libpcsxcore/cdrom.h b/libpcsxcore/cdrom.h
index 860930b..2ec1054 100644 (file)
--- a/libpcsxcore/cdrom.h
+++ b/libpcsxcore/cdrom.h
@@ -91,7 +91,7 @@ typedef struct {
        int CurTrack;
        int Mode, File, Channel;
        int Reset;
-       int RErr;
+       int NoErr;
        int FirstSector;
 
        xa_decode_t Xa;

diff --git a/libpcsxcore/plugins.h b/libpcsxcore/plugins.h
index 132df90..e3bffc7 100644 (file)
--- a/libpcsxcore/plugins.h
+++ b/libpcsxcore/plugins.h
@@ -123,7 +123,7 @@ typedef long (CALLBACK* CDRopen)(void);
 typedef long (CALLBACK* CDRclose)(void);\r
 typedef long (CALLBACK* CDRgetTN)(unsigned char *);\r
 typedef long (CALLBACK* CDRgetTD)(unsigned char, unsigned char *);\r
-typedef long (CALLBACK* CDRreadTrack)(unsigned char *);\r
+typedef boolean (CALLBACK* CDRreadTrack)(unsigned char *);\r
 typedef unsigned char* (CALLBACK* CDRgetBuffer)(void);\r
 typedef unsigned char* (CALLBACK* CDRgetBufferSub)(void);\r
 typedef long (CALLBACK* CDRconfigure)(void);\r