replays bug(s)

Open-source port of PoP that runs natively on Windows, Linux, etc.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: replays bug(s)

Post by Norbert »

Also, does the "(none)" under the "Browse..." portion change to something else when you've selected a file?

And, was the name of the file you tried to upload "01.p1r"?
User avatar
oitofelix
Wise Scribe
Wise Scribe
Posts: 227
Joined: February 17th, 2016, 1:59 pm
Location: Brazil
Contact:

Re: replays bug(s)

Post by oitofelix »

Norbert wrote:I just tried to upload the file in the production environment (the live website), and for me it works.
Indeed, I could fetch the file you uploaded.
Norbert wrote:Can you tell me what exactly you did?
I chose the option "lvl1" in the choose box, and then clicked "Browse" button, then changed "*.pr1" to "All files" in the file selection dialog box, selected a local version of 01.mrp, clicked in "Open" button of that dialog, and then clicked in "Add" button at the web page.
Norbert wrote:Also, can you retry adding it.
Have tried and obtained the same 404 result.

PS: By the way the site never keeps me logged in, even I requesting it to do so.
Bruno Félix Rezende Ribeiro (oitofelix)
MININIM author
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: replays bug(s)

Post by Norbert »

oitofelix wrote:[...], then changed "*.pr1" to "All files" [...] selected [...] 01.mrp
Okay, that pretty much explains it. I renamed 01.mrp to 01.p1r before uploading. So there's probably an abort missing in the part between where the code doesn't accept files other than *.p1r and where it saves the information in the database. I'll look at that, plus I still need to make these changes that accept *.mrp on the client _and_ server side.

Not staying logged-in: I also have that issue, it's something I need to look into one day.
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: replays bug(s)

Post by Falcury »

oitofelix wrote:As I announced here, replays have been added to MININIM. For this feature in action see here. This post is intended to describe MININIM's replay file format and give a rationale for it.
Nice work!
oitofelix wrote:Replay files should carry the absolute minimum information about the engine state (usually only what can be provided by user configuration), and the engine must ensure it can reliably record and reproduce the derivative states in all applicable situations, not allowing the user to interfere in any way with the results (by subsequent reconfiguration of the engine using command line or key bindings, let's say).
Yes, this is also the reason for including many of the configuration options into the SDLPoP replay format. Some of the bug fixes may be very subtle and almost never noticeable... however, they can all be individually configured; and, if the configuration at recording time doesn't exactly match the configuration while replaying, this could cause a replay to become unreproducible.
oitofelix wrote:Doesn't exclude the possibility of movements using two directions in the same axis. MININIM implements smooth command-movement transition, and heavily uses such feature for the player's convenience. As I understand it that would be impossible using SDLPoP's move encoding.
Yes, for SDLPoP it made sense to encode it this way, because it reflects the way movement is handled in the game.
oitofelix wrote:Save states are inherently non-portable. It's prohibitively difficult to make save state information shareable across different sets of architectures (wideness and endianness), after all they are basically a memory snapshot (a very architecture dependent thing), and it's difficult to ensure stability across different releases --- a relatively minor change to the engine may have profound implications on how save states are represented.
You're right of course... Although for SDLPoP, endianness isn't an acute issue for savestates, because the program is not prepared to handle big-endian architectures anyway (it even gives an #error in types.h).
User avatar
oitofelix
Wise Scribe
Wise Scribe
Posts: 227
Joined: February 17th, 2016, 1:59 pm
Location: Brazil
Contact:

Re: replays bug(s)

Post by oitofelix »

Falcury wrote:Nice work!
Thank you. :)
Falcury wrote:Yes, this is also the reason for including many of the configuration options into the SDLPoP replay format. Some of the bug fixes may be very subtle and almost never noticeable... however, they can all be individually configured; and, if the configuration at recording time doesn't exactly match the configuration while replaying, this could cause a replay to become unreproducible.
My experience with replays shows that ensuring reproducibility (particularly across releases) is a very delicate and subtle art. Working on this made me realize the intricacy and complexity of the issue --- things I hadn't been aware before.
Falcury wrote:Although for SDLPoP, endianness isn't an acute issue for savestates, because the program is not prepared to handle big-endian architectures anyway (it even gives an #error in types.h).
What about wideness? Are SDLPoP's save states shareable across different widenesses?
Bruno Félix Rezende Ribeiro (oitofelix)
MININIM author
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: replays bug(s)

Post by David »

oitofelix wrote:What about wideness? Are SDLPoP's save states shareable across different widenesses?
If you mean 32-bit and 64-bit, that was solved here: https://github.com/NagyD/SDLPoP/commit/ ... e2032474bd
User avatar
oitofelix
Wise Scribe
Wise Scribe
Posts: 227
Joined: February 17th, 2016, 1:59 pm
Location: Brazil
Contact:

Re: replays bug(s)

Post by oitofelix »

David wrote:
oitofelix wrote:What about wideness? Are SDLPoP's save states shareable across different widenesses?
If you mean 32-bit and 64-bit, that was solved here: https://github.com/NagyD/SDLPoP/commit/ ... e2032474bd
I see those are save state meta-information within replays. I was really thinking about the save state itself. Quickly looking at SDLPoP's source code, it seems the relevant variable is savestate_buffer (replay.c) which is an array of bytes. How is it exactly used? Is all information contained therein stored and retrieved in fields of fixed width?
Bruno Félix Rezende Ribeiro (oitofelix)
MININIM author
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: replays bug(s)

Post by Falcury »

oitofelix wrote:I see those are save state meta-information within replays. I was really thinking about the save state itself. Quickly looking at SDLPoP's source code, it seems the relevant variable is savestate_buffer (replay.c) which is an array of bytes. How is it exactly used? Is all information contained therein stored and retrieved in fields of fixed width?
The savestate fields all have fixed width types if I remember correctly... (Though now that you mention it, I am not fully sure that the struct padding is fixed for all struct fields...) The relevant procedure is quick_process() in seg000.c. It is called from quick_load() and quick_save() in seg000.c, as well as restore_savestate_from_buffer() and savestate_to_buffer() in replay.c.
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: replays bug(s)

Post by Falcury »

In this commit, I fixed replays not ending properly upon reaching the Princess room:
https://github.com/NagyD/SDLPoP/pull/12 ... f940e4ac1b

I also tweaked some other replay-related stuff.
See these pull requests:
https://github.com/NagyD/SDLPoP/pull/120
https://github.com/NagyD/SDLPoP/pull/121
  • Show error message in a dialog box if replay loading failed
    To make it more obvious what went wrong if you double-click a replay file, we can use SDL_ShowSimpleMessageBox to display the error message using a system dialog. (Should be useful, because not everyone would always look at the console output I think...)
  • Don't play end level music while recording or replaying
    The end level music causes problems, because the next level is loaded exactly when the music stops. We can avoid this problem by simply not playing the music in replays (both while recording, and while replaying). This also improves the pace of replays -- waiting time between levels is reduced
    This should make replays fully reproducible across multiple consecutive levels.
  • Don't allow quickloading while recording a replay
    Obviously, quickloading would mess up a recording, because the quickload is not captured into the replay.
  • Replay save dialog
    After recording, display a dialog box to ask for the replay filename. (This replaces the automatically generated replay filenames)
    Replayed can also be discarded by pressing Escape. (That closes the dialog box without saving the replay.)
I attached a test replay, where I played through levels 4-14 consecutively. It's not a very clean playthrough by any means. Just to show that it is possible :)
Note: to view the replay without the game running out of sync after the level transitions, it's necessary to apply the change from this commit:
https://github.com/NagyD/SDLPoP/pull/12 ... 76c7fce90c
Attachments
levels 4-14 playthrough.p1r
(15.65 KiB) Downloaded 78 times
save_replay.png
save_replay.png (6.96 KiB) Viewed 2705 times
replay_error_message.png
replay_error_message.png (24.2 KiB) Viewed 2705 times
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: replays bug(s)

Post by Norbert »

Possible bug?
viewtopic.php?p=28537#p28537
prince777 wrote: April 16th, 2020, 11:18 pm Level 11
:?: :?: I played as I was in the picture. But I don't know why SDLPoP didn't record the way I played it. Why? :? :?
11.jpg
Attachments
Level-11,Finish.p1r
(4.43 KiB) Downloaded 63 times
Level-11,2.p1r
(4.32 KiB) Downloaded 56 times
Level-11,1.p1r
(3.85 KiB) Downloaded 58 times
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: replays bug(s)

Post by David »

Norbert wrote: April 16th, 2020, 11:53 pm Possible bug?
viewtopic.php?p=28537#p28537
prince777 wrote: April 16th, 2020, 11:18 pm Level 11
:?: :?: I played as I was in the picture. But I don't know why SDLPoP didn't record the way I played it. Why? :? :?
11.jpg
Apparently SDLPoP can't load the guard sprites for level 11.
(SDLPoP writes this to the console: Can't load sprites from resource 750.)
It's probably not related to replays, the same happens if I simply start level 11 with prince.exe megahit 11.

There is no palette (res750.pal) in the FAT.DAT of this mod.
Apparently the original game can handle this, but SDLPoP can't.
(I remember I recently ran into this, but I can't find it in the forum.)
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: replays bug(s)

Post by Norbert »

David wrote: April 18th, 2020, 12:28 pmApparently the original game can handle this, but SDLPoP can't.
To me that seems strange, given your use of the disassembly.
I believe you, of course. Did you perhaps make stubs of certain sections?
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: replays bug(s)

Post by David »

Norbert wrote: April 18th, 2020, 5:17 pm
David wrote: April 18th, 2020, 12:28 pmApparently the original game can handle this, but SDLPoP can't.
To me that seems strange, given your use of the disassembly.
I believe you, of course. Did you perhaps make stubs of certain sections?
This is one of the cases where SDLPoP does things slightly differently from the original game.

Here: https://github.com/NagyD/SDLPoP/blob/ma ... 009.c#L415
SDLPoP loads the palette into a newly allocated area.
It then checks whether the load was successful. If it wasn't then SDLPoP does not load any images.

Code: Select all

	dat_shpl_type* shpl = (dat_shpl_type*) load_from_opendats_alloc(resource, "pal", NULL, NULL);
	if (shpl == NULL) {
		printf("Can't load sprites from resource %d.\n", resource);
		//if (quit_on_error) quit(1);
		return NULL;
	}
The original game instead loads the palette into a local variable, and it doesn't check whether it was successful.
It can be reproduced in SDLPoP if you replace the code above with this code:

Code: Select all

	dat_shpl_type shpl_;
	dat_shpl_type* shpl = &shpl_;
	load_from_opendats_to_area(resource, &shpl_, sizeof(shpl_), "pal");
The result is that if the palette cannot be loaded, then shpl_ will not be changed, and PoP will use whatever (uninitialized junk) data is in it.

If you look at seg009.c of very first release of SDLPoP, you'll see that SDLPoP used to do what PoP DOS does:

Code: Select all

chtab_type* __pascal load_sprites_from_file(int resource,int palette_bits,byte* pack,int shift) {
#if 0
	chtab_type* chtab_ptr;
	dat_shpl_type area;
[...]
	load_from_opendats_to_area(resource, &area, 0);
	pal_ptr = &area.palette;
Post Reply