Opened 2 months ago

Closed 6 days ago

#15358 closed defect (fixed)

SCI: SQ6: GMM Saving not loadable

Reported by: tsoliman Owned by: bluegr
Priority: blocker Component: Engine: SCI
Version: Keywords: SCI32
Cc: Game: Space Quest 6

Description

In a certain game state (savegame included) saving from the in-game save option works fine and loads fine but saving from the GMM works fine but crashes on load.
Occurs in both 1.8.1 and master (539bbd3964)

Steps to reproduce:

  • Load attached savegame with Space Quest 6 from the "Space Quest 6 Collection Series"
  • Save from GMM
  • Load the just created save
  • Crash

ERROR: [sq6-1-1.11 260/0 kAddScreenItem @ 0acb]: kAddScreenItem: Plane 000f:081d not found for screen item 0028:2c92!

Backtrace:

Call stack (current base: 0x0):
 0: script 0 - SQ6::replay()
     obj@0001:29fb pc=0001:0acb sp=ST:0008 fp=ST:0000 argp:ST:0001
 1:[0]  kAddScreenItem(0028:2c92)
     by 0 obj@0000:0000 pc:none argp:ST:0008

Attachments (1)

sq6-1.013 (51.7 KB ) - added by tsoliman 2 months ago.

Download all attachments as: .zip

Change History (8)

by tsoliman, 2 months ago

Attachment: sq6-1.013 added

comment:1 by tag2015, 2 months ago

Summary: SQ6: GMM Saving not loadableSCI: SQ6: GMM Saving not loadable

comment:2 by sluicebox, 2 months ago

Saving SCI games from GMM is an unsafe feature that never should have shipped. It should be removed immediately because it can never be fixed, and even if it could, there is no one here who will do so. Meanwhile, users suffer. Don't ever use it if you want to restore your saved games.

I feel like I'm on a string of negative postings here. But this is an unsafe feature that I've called out ever since it's existence, because I'm the one who dealt with the immediate bug reports years ago. I'm pretty sure I've done the most to fix some of its brokenness. But the more you know about SCI, the more you know that you can't do this feature safely. I don't think SCI GMM saving will ever be made safe, and since it ruins games and actively hurts users, it should be removed immediately. That is the only fix for this bug.

Do you disagree? Prove me wrong =) Otherwise let's shut this down because we're knowingly shipping broken software that ruins games. I just don't have the energy; playing defense drains a lot of joy from this hobby, and PR'ing a feature removal is confrontation and at that point we need to talk about my rates.

Here's someone else who has their game ruined recently, but it happens so often i've lost count: #15250

comment:3 by eriktorbjorn, 2 months ago

"Do you disagree? Prove me wrong =)"

Wouldn't dream of it! :-)

But I'm curious. Is the problem caused by using the ScummVM save/load dialogs instead of the original, or is it caused by saving or loading from the Ctrl+F5 menu? Is it save to load savegames from the ScummVM launcher?

If removing saving (and loading?) from Ctrl+F5 solves the problem, that seems like a no-brainer to me. Just do it.

Being able to use ScummVM's save/load dialogs, though... don't get me wrong, I really like the original dialogs, but if I try to play a game on my phone they can be a bit fiddly to use. (Arguably many Sierra games are very phone-unfriendly to begin with, but sometimes I just want to listen to the music. Don't judge me!) I also seem to recall from my DOSBox days that the original dialogs having a much more limited number of save slots, but if so that can perhaps be dealt with?

Version 0, edited 2 months ago by eriktorbjorn (next)

comment:4 by eriktorbjorn, 8 weeks ago

On the assumption that it's just saving from the GMM that's broken, I guess the good news is that it's still a somewhat recent (January 2022) addition. The number of commits related to it that would have to be reverted is probably fairly small. But of course it's also possible that some of them are robustness fixes that are still good to have, so that's not something I can just do blindly.

I assume it's unsafe for all SCI games, not just some?

comment:5 by somaen, 7 days ago

Priority: normalblocker

This must be fixed for the 2.9.0 release

comment:6 by Filippos Karapetis <bluegr@…>, 6 days ago

In 609e8b54:

SCI: Disallow saving from the GMM and add a new game option to allow it

This disables the feature, but allows users to enable it, if they want,
via a new checkbox. An appropriate warning has been added to the
checkbox, notifying the user that saves created through the GMM may be
corrupted and unusable. This addresses bug 15358

comment:7 by bluegr, 6 days ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.