Menu

#318 ALL: Check for presence and validity of external data files

open
nobody
None
5
2006-07-17
2006-07-11
clem
No

- damaged scummvm datafiles prevent games from playing
- scummvm gives only cryptic information when the
datafiles are damaged

after running into a similar problem with sky.cpt, I
think joostp added a size check which would complain if
the datafile was not present in its entirety - maybe
add that to ScummVM as well?

here's the problem with kyra on the forum
http://forums.scummvm.org/viewtopic.php?t=1784&highlight=

Discussion

  • clem

    clem - 2006-07-11
    • assigned_to: nobody --> lordhoto
     
  • Max Horn

    Max Horn - 2006-07-12

    Logged In: YES
    user_id=12935

    Definitely sounds like a good idea. We should do that for all the external data
    files, i.e.

    kyra.dat
    lure.dat
    queen.tbl
    sky.cpt

    We should first check for the presence of the file, and if missing, show an
    approriate error *dialog*. Next, run some simply integrity check on the files
    (minimal file size, a fixed header, whatever), and if that fails, display
    *another* dialog, telling that the data file is likely corrupt.
    And ideally, a third dialog would be shown if the data file was outdated.

     
  • Max Horn

    Max Horn - 2006-07-12
    • summary: kyra: check size of kyra.dat --> ALL: Check for presence and validity of external data files
     
  • Eugene Sandulenko

    Logged In: YES
    user_id=166507

    data file structure:

    TAG VER LENGTH data block MD5

    I.e.:
    1. Check TAG that file is valid
    2. Check version
    3. Check LENGTH
    4. Calculate MD5 and match

    This will be easy to implement and will be universal enough
    to avoid any hardcoded file details.

     
  • clem

    clem - 2006-07-12

    Logged In: YES
    user_id=1138674

    wouldn't that require an update to all data files?

    I think lordhoto once mentioned that kyra.dat is the same
    format as the kyra .pak files

     
  • Eugene Sandulenko

    Logged In: YES
    user_id=166507

    Yes, but that will be done only once and will save us from
    these periodic bogus bugreports.

     
  • Johannes Schickel

    Logged In: YES
    user_id=991728

    The current Kyrandia implementation hardcodes the size in
    the ScummVM sourcecode, if someones implements the described
    'data file structure' in a generic way I'll change the
    Kyrandia code to support it. I'm changing the 'Assigned To'
    field to None for now, since I'm not working on a generic
    way, so any other developer can feel free to implement it
    and as I, said before, will change the Kyrandia code then.

     
  • Johannes Schickel

    • assigned_to: lordhoto --> nobody
     
  • Johannes Schickel

    Logged In: YES
    user_id=991728

    I would use a structure like this though:
    TAG \ (maybe this field could be engine depended too)
    VER | Datafile header
    SIZE |
    MD5 /

    DATAFILE > Engine specific stuff

    I don't see much sense behind adding the MD5 at the file end.

     
  • Oystein Eftevaag

    Logged In: YES
    user_id=1025462

    We could also preserve at least some measure of backwards
    compatability by trying to load the file as the old format
    if it fails the integrity check, and just display a 'Your
    copy of kyra.dat may be outdated or corrupted' message
    (given that the current format of kyra.dat would use invalid
    fields for the integrity check, and always fail it).

    It doesn't really make sense to force people to download a
    new version of kyra.dat for an integrity check when they
    know their current one works fine :).

     
  • clem

    clem - 2006-07-24

    Logged In: YES
    user_id=1138674

    maybe simply add a size-check for now, add the more
    sophisticated check the next time kyra.dat changes (or if it
    is stable, with the next stable version?)

     
  • Johannes Schickel

    Logged In: YES
    user_id=991728

    Clemmy: we have that size check by now.

    We should just try to load kyra.dat/otherdatafile in the old
    style way if just the TAG check failed, we should add a
    return value in the loading function which tells what failed
    while trying to load it the new way IMHO.