Opened 13 years ago

Closed 13 years ago

Last modified 3 months ago

#5714 closed defect

SWORD25: Saving fails

Reported by: fingolfin Owned by: sev-
Priority: normal Component: Engine: Sword25
Version: Keywords:
Cc: Game: Broken Sword 2.5

Description

Started a new game with latest master version, and once I was able to control the main actor, I tried to make a save. This resulted in ScummVM erroring out with this message:

Request to persist a dynamic bitmap (video) - probably a bug!

Ticket imported from: #3307396. Ticket imported from: bugs/5714.

Change History (15)

comment:1 by fingolfin, 13 years ago

Just made another attempt, and this time it worked. Then, I immediately tried again to save (I didn't even leave the save screen after the first save succeeded), and this time I got another error message: Error while reading PNG image!

It seems the second save was corrupt, because after it was created, the game now always crashes when I go to the Load or Save screens.

However, I think that this might be a different bug than the one that made be file this bug report initially.

comment:2 by fuzzie, 13 years ago

I guess I was the last person to touch this code? But latest master still works fine for me, on x86 and ppc Linux (with the German version).

comment:3 by fingolfin, 13 years ago

Weird. Well, for the record: Using a build made from rev g7b51cae, the English version of Sword25, running on Intel Mac OS X.

I can always reproduce the issue: First I make a save, then without leaving the savescreen I immediately make a second save; this then triggers "Error while reading PNG image".

The second save seems to be much smaller, too, as if it had failed to write parts...

-rw-r--r-- 1 mhorn staff 490121 May 26 11:13 sword25.001 -rw-r--r-- 1 mhorn staff 445733 May 26 11:13 sword25.002

If it helps, I can email the good and broken savegames.

Oh, and I thought this might be related to the recent PNG changes... I just see that Filippos made some more changes there, so I'll retry with the very latest master.

comment:4 by fingolfin, 13 years ago

Owner: bluegr removed

comment:5 by fingolfin, 13 years ago

I think the SHA-1 I gave in my previous comment was nonsense; anyway, I just reproduce the issue with commit a654115f1a9

comment:6 by fingolfin, 13 years ago

And since I just this mentioned in the IRC logs: I did *not* overwrite existing savestates with my tests (that was my first suspicion, actually, so i deleted all existing saves).

Regarding the original bug in this post: I still could not reproduce it, but I wonder if it might be a race condition? Are videos only played for cutscenes, or also for sub-screen animations?

comment:7 by bluegr, 13 years ago

My commits did not touch this part of the code yet.

It might be a race condition, as you say. I put the error for the first bug you mentioned there - videos cannot be persisted, so this should never happen. Quite odd, indeed.

As for the second bug: again, not quite sure what's wrong yet, I'm afraid :/

Will have a look in the afternoon.

comment:8 by fuzzie, 13 years ago

Huh, I can reproduce the second bug (with the PNG error) now. Will take a look if Filippos doesn't fix it first.

comment:9 by fuzzie, 13 years ago

Second bug fixed in 7b03a6e6.

comment:10 by digitall, 13 years ago

What is the status of this bug? I think that all the reported issues have been fixed, though it may be open as a reminder to rewrite the BS25 save code to remove the non-portable Pluto system.

If so, I'd suggest closing this as fixed and moving that TODO to the relevant wiki page i.e. http://wiki.scummvm.org/index.php/Sword25/TODO

comment:11 by sev-, 13 years ago

Status: newclosed

comment:12 by sev-, 13 years ago

Right, closing this. The engine requires savesystem rewrite.

comment:13 by sev-, 13 years ago

Owner: set to sev-

comment:14 by fingolfin, 13 years ago

Hm, seems I still get notifications for this item.

The issue I reported here has *not* been fixed to my knowledge, at least no comment here claims to have fixed it. Only the second issue, reported in my first comment to this tracker item, was fixed. But I didn't test whether it still occurs or not -- just saying that I don't have the impression anybody tried it either.

comment:15 by Filippos Karapetis <bluegr@…>, 3 months ago

In b5e79d9d:

TETRAEDGE: Cache FSNode lookups

This is based on @andrewglass's work in PR #5714, and allows us to
cache the remaining cases where Common::FSNode is used

Note: See TracTickets for help on using tickets.