hiscore saving

[ghost]

I was speaking to @barbudreadmon on the retropie forums and he told me to post the issue here. There is probably a good reason for whats going on so thought i would ask.

the code here

FBNeo/src/burn/hiscore.cpp

Lines 150 to 158 in db14c31

	static INT32 CheckHiscoreAllowed()
	{
	INT32 Allowed = 1;
	if (!EnableHiscores) Allowed = 0;
	if (!(BurnDrvGetFlags() & BDF_HISCORE_SUPPORTED)) Allowed = 0;
	return Allowed;
	}

has two conditions to set hi scores the problem is if you enable hi scores and its in the dat the driver flag check will stop it working even if this option is enabled.

Is there a reason for this or could adding and if (!(BurnDrvGetFlags() & BDF_HISCORE_SUPPORTED) && !EnableHiscores ) Allowed = 0; be used or does every driver need updated

[dinkc64]

You have a good idea, but the problem is that the drivers need to have HiscoreReset() called on reset,
There is a design flaw in fbneo where the reset function is completely local to the driver. Meaning, only the driver itself knows when it's been reset...

If we tweaked hiscore.cpp to allow all drivers to have hiscore as long as they're in the dat: let's say if the user presses F3/Reset and HiscoreReset() is not called, then they close the emulation, all their hiscore data will be lost as the game loads the defaults on reset.

I might be able to hook the input and call HiscoreReset from there. kinda hacky, but, the only other option is to add HiscoreReset() to all existing drivers that are missing it.
Let me think about this, perhaps I can conjure up a good / better idea.

[dinkc64]

I came up with another idea "ignore the driver flags"
basically, we'll detect if HiscoreReset() was called. If it was called by the driver, then hiscores will be enabled (if they're in the dat, and hiscore enabled in options, of course)

[ghost]

well i was going to suggest to add a global variable and set say found_cheat when processing the dat file. to check if a cheats was found and check for this variable on reset. You know the code better than me though I was just originally just going to send a message saying a cheat was found add flag to driver

[barbudreadmon]

I came up with another idea "ignore the driver flags"

Good idea ! Removing the BDF_HISCORE_SUPPORTED requirement seems pretty harmless, and would nicely speed-up the process of enabling hiscore in a driver.

[dinkc64]

barbudreadmon, thanks, I'll try to impliment it today.

Just something to remember (and sorry in advance for the lecture...)
It's always difficult changing core components like this. You can't just think "This will be a nice feature/benefit". It's more in the line of "what's going to break when I add this feature" - like I said earlier, if we just flat out ignored the driver flags (and no check for "if HiscoreReset() was called in the driver..", it would be disastrous - the highscores would get saved, but if the end-user reset the game, the highscores would get overwritten by defaults.. eek.

[Riddler1]

Hey Dink, is the BDF_HISCORE_SUPPORTED funtion scrap now or can i submit changes for games i have tested and made?

[barbudreadmon]

BDF_HISCORE_SUPPORTED is still in use

[Riddler1]

👍Thanks

[eadmaster]

What if we had an additional check in HiscoreOkToWrite() to verify if the values in hiscore ranges were re-inited again? This way we can detect a reset without modifing the game drivers.

I'm still studying the code, but look at this, hopefully you get the idea:

INT32 HiscoreOkToWrite()
{ // Check if it is ok to write the hiscore data aka. did we apply it at least AND was not reinited?
	INT32 Ok = 0;
	
	for (UINT32 i = 0; i < nHiscoreNumRanges; i++) {
		if HiscoreMemRange[i].Loaded && HiscoreMemRange[i].Applied == APPLIED_STATE_CONFIRMED) {
			// check reinit
			cpu_open(HiscoreMemRange[i].nCpu);
			for (UINT32 j = 0; j < HiscoreMemRange[i].NumBytes; j++) {
				if (cheat_subptr->read(HiscoreMemRange[i].Address + j) == HiscoreMemRange[i].Data[j]) {
					Ok = 0;
				} else {
					Ok = 1;
					break;  // found 1 diff value, ok to write 
				}
		}
	}
	cheat_subptr->close();

[dinkc64]

It would be a good idea, but the data is not deterministic enough to count on that to detect a reset.
For example: some games clear their memory on reset, some don't. Sometimes we clear memory on reset, sometimes we don't (depending on the hw / situation). The only surefire way is to call "HiscoreReset();" in the reset handler.

best regards,

• dink

[eadmaster]

For example: some games clear their memory on reset, some don't.

If they don't then there is no harm in saving the same data again in the hiscore file, because is still valid (no data loss).
The real part i'm still missing is to reinit the hiscore ranges after a memory reset+clear.

Btw i definitively see your point in avoiding hackish solutions... but i'd still like to do some experiments with this, do you know a game that does these self soft-resets?

[barbudreadmon]

We won't remove the BDF_HISCORE_SUPPORTED requirement, as proven with d49b309 , enabling hiscore.dat for some games is breaking them, so we need a mean to toggle it off.

Also, we don't think it's a FBNeo issue, from looking at mame debugger, those address ranges for astdelux in hiscore.dat are very suspicious (game timers ?), there are also concerning comments about them in hiscore.dat, and the game doesn't need this to save hiscores (earom is handling this already).