dirkwhoffmann / vAmiga

vAmiga is a user-friendly Amiga 500, 1000, 2000 emulator for macOS
https://dirkwhoffmann.github.io/vAmiga
Other
298 stars 25 forks source link

Disk change is recognized much better now. But maybe still not 100% reliable? #156

Closed mithrendal closed 5 years ago

mithrendal commented 5 years ago

Yesterday I tried "desert strike" (3 Disks) Which is an absolutely cool title BTW.

Some weeks before it simply would not recognize its disk2. The game commanded me to "insert disk2", which I carried out, but the game did not recognized the disk change. But yesterday with the latest vAmiga it worked through like a charme. Only when it asks for disk3 then it did not recognize the disk change.

Then I did something very stubborn and stupid... I kept on inserting disk3 maybe 5 times or so and then eventually it worked at the 6th attempt. At the 5 unsuccessfully times, although vAmigas screen did do the "flying in" animation, the game itself has not recognized the diskchange.

Then the game has been fully playable. A nice big helicopter moved absolute fluently by the blitter only (as I could see from the DMA visualiser).

Then I stopped playing and did some more testing.

Test-result: This game run in vAmiga seems to accept disk changes but only 1 out of 3-6 maybe on a random basis.

TODO: I still have to crosscheck with SAE... TODO: I still have to check the disk change in other multiple disk games...

PS: I had to outcomment an assertion("...") to get past the intro...

dirkwhoffmann commented 5 years ago

This game run in vAmiga seems to accept disk changes but only 1 out of 3-6 maybe on a random basis.

The indeterministic behaviour strongly indicates that the delay between ejecting and inserting disks is too short (vAmiga switches disks in 0.5 seconds which is probable too quick).

To validate this assumption, simply eject the disk manually (via the Df0 menu) before inserting the new one. If this works fine, my hypothesis is right.

I had to outcomment an assertion("...") to get past the intro...

I observe that asssertion failures are responsible for 99% of all crashes. Maybe I better take 'em all out πŸ€”.

I am a little bit distracted at the moment, because I have to solve a very tuff bug. The bathroom door is broken and I need to replace the feather of the closing mechanism. This sounds simple, but to get to the feather I need to remove to lock. And to remove the lock, I need to remove the rose which is clipped over the locked. I unsuccessfully tried with a screwdriver before I learned that I need a special tool for that 😳.

https://www.amazon.de/HEWI-Kappenheber-Rosetten-Schildkappen-StΓΌck/dp/B075CTDDWZ

Lesson learned:

mithrendal commented 5 years ago

I am a little bit distracted at the moment, because I have to solve a very tuff bug.

But these roses (at least mine) do have a little notch at the very bottom. There you could try again maybe with the screwdrewer. No? If you scratch there, this is not visible because you don't spot the bottom of a rose unless you are lying on the ground...

To validate this assumption, simply eject the disk manually (via the Df0 menu) before inserting the new one. If this works fine, my hypothesis is right.

I will test this...

I observe that asssertion failures are responsible for 99% of all crashes. Maybe I better take 'em all out.

For the development/test version it is helpful to have all the assertions active, because it gives hints where the emulation runs ... lets say a bit uncontrolled ....

But for a "gaming version" I agree with you, that you don't want to have them in.

dirkwhoffmann commented 5 years ago

Maybe I better take 'em all out.

I was just joking. I would never ever remove my beloved assertions πŸ˜‰.

But for a "gaming version" I agree with you, that you don't want to have them in.

This is handled in file config.h:

//
// Release settings
//

// Snapshot version number
#define V_MAJOR 0
#define V_MINOR 1
#define V_SUBMINOR 0

// Assertion checking (uncomment in a release build)
// #define NDEBUG

In C / C++, assertion checking can simply be disabled by defining preprocessor constant NDEBUG. In release builds, I usually take all assertions out by uncommenting the last line shown above. For all other builds, I keep assertions active, because I wouldn't get useful debug information otherwise.

mithrendal commented 5 years ago

The indeterministic behaviour strongly indicates that the delay between ejecting and inserting disks is too short (vAmiga switches disks in 0.5 seconds which is probable too quick). To validate this assumption, simply eject the disk manually (via the Df0 menu) before inserting the new one. If this works fine, my hypothesis is right.

I tested twice to fully load the game with the manual eject method above. And both times it recognized the disks at once. Hypothesis is valid!πŸ‘πŸ» What should I do next ? Can we also get it "deterministic" without manually ejecting the disks?πŸ€”

dirkwhoffmann commented 5 years ago

Can we also get it "deterministic" without manually ejecting the disks?

Yes, sure. Inserting a disk is a scheduled event and we can adjust the time between ejecting the old disk and inserting the new one arbitrarily. The old implementation waited for 1 second (not for 0.5) which was obviously not enough. I just changed it to 1.5 seconds:

Here is the code:

void
DiskController::insertDisk(class Disk *disk, int nr, Cycle delay)
{
    assert(disk != NULL);
    assert(nr >= 0 && nr <= 3);

    debug(DSK_DEBUG, "insertDisk(%p, %d, %d)\n", disk, nr, delay);

    amiga->suspend();

    if (df[nr]->hasDisk()) {

        // Eject the old disk first
        df[nr]->ejectDisk();

        // Make sure there is enough time between ejecting and inserting
        delay = MAX(MSEC(1500), delay);
    }

    diskToInsert = disk;
    agnus->scheduleRel<DCH_SLOT>(delay, DCH_INSERT, nr);

    amiga->resume();
}

The new code shouldn't require you to disable assertions. I've fixed some relates crashes, too.

dirkwhoffmann commented 5 years ago

Hmm, with a 1.5 seconds delay, I had one failing disk change (out of 4 I think). I could increase the delay to 2, but if we select a too large value, it will affect user experience of other programs.

Anyway, the game has a really cool intro 😎.

Bildschirmfoto 2019-09-04 um 11 10 45

Seatbelts fastened. Ready to get the party started πŸ₯³.

Bildschirmfoto 2019-09-04 um 11 13 14

Hmmm, no compass, no map, no fuel indicator. Where shall I go 😬?

Solved. Land ahoy! 😎

Bildschirmfoto 2019-09-04 um 11 18 08

All of a sudden, my combat helicopter started taking to me. What exactly could "Warning - Danger zone" mean πŸ€”?

Oups 😳

Bildschirmfoto 2019-09-04 um 11 21 20
mithrendal commented 5 years ago

anywhere in the game press 0 then it takes you to the full status information HUD

image

and it shows you where you have to rescue the people....

You can choose what you are looking for. For example radar sites. Humans to rescue...

...and hopfully they let you later dock to a harbour to let all the rescued humans on a save EU land....πŸ™„

dirkwhoffmann commented 5 years ago

you have to rescue the people....

This is a rescue mission? Oups 😳. Maybe, I shouldn't have skipped the intro where they explain what I have to do 😬. I shot anything that dared move πŸ™„.

Anyway, I think I keep the 1.5 seconds disc change delay for now. If the problem is due to something else (not the delay), other tests will eventually reveal it. At the moment, it's seems most promising to me to debug the issues the disk copy programs have.