[Scummvm-tracker] [ScummVM :: Bugs] #15931: SCUMM: ZAK: Out-of-bounds charset error with v0.13.x saves

[ScummVM :: Bugs]

#15931: SCUMM: ZAK: Out-of-bounds charset error with v0.13.x saves
--------------------------+---------------------------
Reporter:  dwatteau       |      Owner:  (none)
    Type:  defect         |     Status:  new
Priority:  normal         |  Component:  Engine: SCUMM
 Version:                 |   Keywords:  saves
    Game:  Zak McKracken  |
--------------------------+---------------------------
 I have some Zak V2 saves from 2009 which cause a fatal error ("`ERROR:
 charset 83951879 is out of bounds (0,8)!`") with modern ScummVM.

 They're `VER(77)`, which means they were made with ScummVM 0.13.x.

 I can't find the exact reason for it, but I mostly get what's going on,
 and I've found some context.

 It may have been introduced by:

 * this old commit:
 <https://github.com/scummvm/scummvm/commit/3047e084c1402af4f7643b718e655532c0e2c892>
 * and/or may be related to older Trac#3914 and Trac#3703.

 i.e. at some point around the v0.13.0 release, `_curId` was introduced and
 then bumped from a byte to uint32 (we have some code dealing with such
 cases). And then, there possibly was an issue with `_curId` not always
 being properly initialized for v1-v3 titles.

 This diff fixes the issue for me:

 {{{
 diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
 index e30ab7e1f9b..937902e7ac6 100644
 --- a/engines/scumm/charset.cpp
 +++ b/engines/scumm/charset.cpp
 @@ -1056,6 +1056,13 @@ void
 CharsetRenderer::saveLoadWithSerializer(Common::Serializer &ser) {
         ser.syncAsByte(_color, VER(73));

         if (ser.isLoading()) {
 +               // Some old v0.13.x saves have bogus values, for some
 reason (see
 +               // bug #15930). When detecting such weird values made
 before the
 +               // v1.0.0 release (VER(80)) that followed it, reinitialize
 the id
 +               // using a, hopefully, sane value.
 +               if (ser.getVersion() < VER(80) && _curId >
 _vm->_numCharsets - 1)
 +                       _curId = _vm->_string[0]._default.charset;
 +
                 setCurID(_curId);
                 setColor(_color);
         }
 }}}

 It reuses what's been done for Trac#3194 in commit
 bd67214c96c1bb1b194cfc31abf05a08afc9cf64.

 I'll submit a PR for it. This ticket is mostly here to document the
 problem (that nobody reported after the v1.0.0 release... possibly because
 it may have only impacted v1-v3 games?). And also to have the saves
 preserved somewhere.
-- 
Ticket URL: <https://bugs.scummvm.org/ticket/15931>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM