libretro / gambatte-libretro

Hard fork of Gambatte to the libretro API.
http://sourceforge.net/projects/gambatte/
GNU General Public License v2.0
105 stars 79 forks source link

Added cheevos via the mmaps interface #50

Closed leiradel closed 8 years ago

Alcaro commented 8 years ago

+ { 0, gb.savedata_ptr(), 0, 0xA000, 0, 0, sramlen, NULL },

We want it mirrored over the entire area. Set select=~0x1FFF (or is it ~0x0FFF?).

+ environ_cb(RETRO_ENVIRONMENT_SET_SUPPORT_ACHIEVEMENTS, &yes);

Since when do cores need to know about frontend features? That just sounds stupid. And the frontend still needs to ask some external data source which console Gambatte is for, to find what achievement set to load, and then the achievement support flag can be placed there instead.

And I want a very clear explanation of under which circumstances a core would support memory maps but not achievements.

+ * This must be called before the first call to retro_run.

That specification makes sense for functions the frontend calls, but not in the other direction. Should be "in retro_set_environment or retro_init".

leiradel commented 8 years ago

We want it mirrored over the entire area. Set select=~0x1FFF (or is it ~0x0FFF?).

Which one? I tried to find info about it but failed. Maybe it must be calculated from sramlen?

Since when do cores need to know about frontend features? That just sounds stupid. And the frontend still needs to ask some external data source which console Gambatte is for, to find what achievement set to load, and then the achievement support flag can be placed there instead.

You got it wrong, it's the other way around. It's for the core to tell the frontend that it supports cheevos. Many cores implement retro_get_memory_data/size and/or mmaps wrongly, which confuses the cheevos module that doesn't know any better and believes that if they return something, it must be correct.

The problem is that the wrong implementation has unwanted collateral effects, like the user receiving awards for cheevos he didn't win at the very beginning of the emulation.

So unless we correct all the cores and make sure the mmaps implementation is correct, or we remove the wrong mmaps and implement retro_get_memory_data/size correctly (assuming cheevos will be able to work with this inferior interface), the core has to explicitly tell the frontend about cheevos support.

That specification makes sense for functions the frontend calls, but not in the other direction. Should be "in retro_set_environment or retro_init".

It is in the other direction.

Alcaro commented 8 years ago

Which one? I tried to find info about it but failed. Maybe it must be calculated from sramlen?

That's not how mirroring works.

http://gameboy.mongenel.com/dmg/asmmemmap.html says cartridge RAM goes from A000 to BFFF. The difference is 1FFF, so use that.

You got it wrong, it's the other way around.

The core is telling the frontend it supports this, therefore the core knows about it.

Many cores implement retro_get_memory_data/size and/or mmaps wrongly

Do you know of any specific core where that is the case, or is it just a suspicion based on our number of cores and RetroArch ignoring that specific function until now?

Either way, core bugs are to be fixed. If we require cores to opt in, nobody will find the bugs, so we can't fix them.

The problem is that the wrong implementation has unwanted collateral effects, like the user receiving awards for cheevos he didn't win at the very beginning of the emulation.

A valid argument, but I'm not convinced it warrants adding crap to the libretro interface. I'd rather fix that by asking the user to report the bug if an achievement is collected in the first 60 frames or whatever.

And I'm not convinced the achievements are that important, anyways.

It is in the other direction.

Yes, and that's the direction in which the description doesn't make sense.

leiradel commented 8 years ago

http://gameboy.mongenel.com/dmg/asmmemmap.html says cartridge RAM goes from A000 to BFFF. The difference is 1FFF, so use that.

Ok.

The core is telling the frontend it supports this, therefore the core knows about it.

Ah, yes, the core knows about it. The core also knows many other things, and tells the frontend how it wants them. I don't see how this one is any different.

Do you know of any specific core where that is the case, or is it just a suspicion based on our number of cores and RetroArch ignoring that specific function until now?

One of the GBA and one of the SNES cores were awarding me a random cheevo right at the start, I don't remember which ones specifically.

Sometimes the mmaps are not wrong but incomplete (i.e. RAM is mapped, VRAM isn't), and it could cause some cheevos to be awarded and others not.

Either way, core bugs are to be fixed. If we require cores to opt in, nobody will find the bugs, so we can't fix them.

The bug is omg, my favorite core doesn't support cheevos! fix it!

A valid argument, but I'm not convinced it warrants adding crap to the libretro interface. I'd rather fix that by asking the user to report the bug if an achievement is collected in the first 60 frames or whatever.

One core was giving @fr500 all cheevos of an specific game, I don't remember the details. I'd rather not wait for that kind of bug report to arrive, because until it arrives and we fix the core, which can take some time, many users will have already exploited it.

Of course the cheevos services are easily exploitable, but we don't need to make cheating any easier.

And I'm not convinced the achievements are that important, anyways.

We're way past that discussion now.

Alcaro commented 8 years ago

and VRAM isn't

Easy fix. Unless VRAM isn't exposed to the primary bus, but then there are no achievements there anyways.

I'm not completely sure that warrants disabling the rest too, but considering who our users are, it's probably the best solution.

One of the GBA and one of the SNES cores were awarding me a random cheevo right at the start, I don't remember which ones specifically.

I don't think that's the libretro interface being buggy. It's probably reacting to the initial RAM values.

If we assume there are 50 achievements, they trigger on one byte having a specific value, and random initial RAM, the chance of getting at least one is 1-(255/256)^50 = about 17%. Every time the game is loaded.

I suspect the actual chance is higher, since many games have a 'beat 10 levels' that trigger on RAM_LevelsBeaten >= 10; the chance of that is 96%. They'd also trigger if RAM is initialized to 0x55, which is quite common. (bsnes randomizes, but I don't know of anything else. I'd guess the original RetroAchievements emus set everything to 00.)

Solution 1: Ignore everything for the first 60 frames. By then, the game should've wiped RAM. Solution 2: Remember if the achievement was accomplished last frame. Hand out the achievement only when that changes.

The bug is omg, my favorite core doesn't support cheevos! fix it!

Good point.

Okay, I propose the following compromise:

It is an ugly hack, but we'll keep it for now; the alternatives are worse.

However, I expect this env removed from the spec once all relevant cores are tested and confirmed working. It's experimental, we can do that.

leiradel commented 8 years ago

It is an ugly hack, but we'll keep it for now; the alternatives are worse.

However, I expect this env removed from the spec once all relevant cores are tested and confirmed working. It's experimental, we can do that.

Agreed.