lanceewing / agile

Sierra AGI (Adventure Game Interpreter) engine written in C#
15 stars 7 forks source link

a couple of save game bugs #26

Closed vafada closed 2 years ago

vafada commented 2 years ago

I believe these are bugs.

When saving the game, state.BlockLowerRightX shouldn't be set.

And the for-loop should probably be

(state.Objects.NumOfAnimatedObjects + 1)

to be consistent with

int aniObjectsLength = ((state.Objects.NumOfAnimatedObjects + 1) * 0x2B);

lanceewing commented 2 years ago

I agree with the BlockLowerRightX change, as that looks like a copy and paste error. You're right that it shouldn't be setting it.

It took me a while to work out the second change, but yes, you're right, the values should definitely match. What I was trying to remember is why the +1 was there in the first place, and this is because the animated object table is always the max number value plus 1. The WinAGI help implies that this is because ego is not included in the count, but actually the comments in the original AGI source code say something different:

    object = (OBJECT *)     LoadFile(objectFileName, object);
    Encrypt(object, (STRPTR) object + segLen);
    objLen = segLen - sizeof(OBJECT);
    numAni = object->room;
    objEndOfs = W8086(object->nameofs);
    ++object;
    objEnd = (OBJECT *) ((STRPTR) object + objEndOfs);

    /* Get the number of animated objects from the room number of the
    ** first entry of the object table, point past that entry to the
    ** objects, and allocate and initialize aniObj.  Add one object at
    ** the end of the table for use by picobj
    */
    ++numAni;

So it is saying that the additional animated object slot at the end is for use by the "picobj", which is another name for add.to.pic. While the interpreter is adding an add.to.pic, it uses the additional animated object in the table for that purpose. AGILE doesn't do this at the moment. Instead it just instantiates an AnimatedObject on the fly and uses that. I guess that if we want to get as close as we can to the original saved game format, then perhaps AGILE should use the appropriate slot in the AnimatedObject list.

Strictly speaking, it doesn't need to, and I don't think the original AGI interpreter needed to save that additional slot either, as the add.to.pic entry in the animated objects table isn't used after it is added to the picture. But it might confuse things, such as saved game file comparisons, if we don't do this, so I think we should match that behaviour.

lanceewing commented 2 years ago

Sorry, didn't mean to close. Was only trying to add the comment.

lanceewing commented 2 years ago

Added Issue #30 to cover changing the add.to.pic command to use an additional slot in the AnimatedObjects array, which would then be included in the saved game file.

AGKorson commented 1 year ago

So it is saying that the additional animated object slot at the end is for use by the "picobj", which is another name for add.to.pic. While the interpreter is adding an add.to.pic, it uses the additional animated object in the table for that purpose. AGILE doesn't do this at the moment. Instead it just instantiates an AnimatedObject on the fly and uses that. I guess that if we want to get as close as we can to the original saved game format, then perhaps AGILE should use the appropriate slot in the AnimatedObject list.

Strictly speaking, it doesn't need to, and I don't think the original AGI interpreter needed to save that additional slot either, as the add.to.pic entry in the animated objects table isn't used after it is added to the picture. But it might confuse things, such as saved game file comparisons, if we don't do this, so I think we should match that behavior.

That might be what it says, but it's not what it does. The WinAGI Help file has it right.

The 'max number of objects' value that comes from the OBJECT file is not the 'total number of objects'; it's the highest object INDEX that's available. The 'max' or 'count' is then 'INDEX+1'. To illustrate, if the value in OBJECT is '16' (common in many games), that means objects o0 through o16 are all valid objects (for a total of 17). That's why the value gets incremented before allocating memory. It's easy to verify this by animating objects up to the max value (or by examining memory within DosBOX). If your save file doesn't use the incremented count you'd be dropping the last valid object. It would rarely be noticed though because few games actually use all the objects that are allocated.

So what's up with the add.to.pic "object"? It's actually located in the data overlay, not on the heap. This is true for all version 2/3 interpreters. In v2.917 for example, it's at DS:0EAC. It does not get included in the save.game file.

The data in the save game file are well defined - the WinAGI Help file lists the exact composition. The gamestate section is what causes the most variation between interpreter versions. Since it's a dump of a section of the data overlay, the information in that section is highly version-dependent. If you are interested in exactly what is in a particular version's saved gamestate, you can check out the IDAPro files I sent. The saved data generally goes from DS:0002 (the GameID field) and runs up to where the "Press ENTER to quit" text string is stored.

lanceewing commented 1 year ago

@AGKorson , Ah, okay. Thanks for clarifying that. I'll close the #30 issue, as it isn't relevant then.

Yes, I am aware of a number of the differences in the game state section, and I think I have quite a few of them supported now. AGILE will try to save a save game file in the format for the AGI version that it thinks the game it is executing was using. We've been able to load AGILE save games in to a number of different versions now, ranging from various v2 interpreters to v3 as well. I did use the information in the WinAGI Help to help with implementing those bits. Not sure if I have it 100% though, but I tried to cover it.

If I compare the save game files though, there are clearly a lot of differences between what AGILE produces and what the AGI interpreter produces, but the size of the saved game file is now a match for the versions I've tried, and it seems to load in okay. There could be subtle bugs still in there though, such as the max object one that you mentioned above. I'm sure there are still things to find and fix in there.