libretro / snes9x2010

Snes9x 2010. Port of Snes9x 1.52+ to Libretro (previously called SNES9x Next). Rewritten in C and several optimizations and speedhacks.
Other
98 stars 71 forks source link

savestate size #84

Open zeromus opened 7 years ago

zeromus commented 7 years ago

Not verified, just pasting from IRC for recordkeeping:

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/37956385-savestate-size?utm_campaign=plugin&utm_content=tracker%2F626376&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F626376&utm_medium=issues&utm_source=github).
Alcaro commented 7 years ago

Yeah, that looks broken.

But I can't find how to fix that thing. There is a memstream_close(), but it takes a stream reference as argument, and memstream_get_last_size() doesn't use anything like that - it pokes some global variables instead.

That entire memstream file is a big mess. If it supports multiple streams, then it shouldn't have those globals at all, it should just require a reference for everything. The globals should be in libretro.c instead.

zeromus commented 7 years ago

I'm glad you agree. It gave me a bad feeling too. I'm disappointed it wasn't as simple as I thought at my first glance. Blrrrrrr

OV2 commented 7 years ago

The stream is already correctly closed. memstream_open was changed at some point to have a "writing" parameter, but the mapping to OPEN_STREAM in snes9x.h always calls it with 0. That is why memstream_close always sets last_file_size to the complete size of the buffer. Changing the define to set the writing parameter depending on the mode should fix it.

Alcaro commented 7 years ago
size_t retro_serialize_size (void)
{
   uint8_t *tmpbuf;

   tmpbuf = (uint8_t*)malloc(5000000);
   memstream_set_buffer(tmpbuf, 5000000);
   S9xFreezeGame("");
   free(tmpbuf);
   return memstream_get_last_size();
}

memstream_open having a writing parameter would help if we actually used that function.

I guess the relevant refactoring was done after whatever version this is based on (1.52, I think?).

OV2 commented 7 years ago

The memstream here was a hack by themaister to get the old s9x version to save to memory instead of a file. The functions are mapped in snes9x.h:

#define STREAM memstream_t *
#define READ_STREAM(p, l, s)     memstream_read(s, p, l)
#define WRITE_STREAM(p, l, s)    memstream_write(s, p, l)
#define GETS_STREAM(p, l, s)     memstream_gets(s, p, l)
#define GETC_STREAM(s)           memstream_getc(s)
#define OPEN_STREAM(f, m)        memstream_open(0)
#define FIND_STREAM(f)           memstream_pos(f)
#define REVERT_STREAM(f, o, s)   memstream_seek(f, o, s)
#define CLOSE_STREAM(s) memstream_close(s)

S9xFreezeGame causes calls to OPEN_STREAM and CLOSE_STREAM, which then ends up in the memory set up by memstream_set_buffer. Something like this might work: #define OPEN_STREAM(f, m) memstream_open(strchr(m, 'w'))