Memory management problem

Joel I was recently messing around in my game shapeshifting into all the different characters that it was possible to shapeshift into and suddenly my game crashed with an out of memory error. This isn't a serious problem, because it's unlikely to happen in practice (I was in debug mode, had changed some flags that prevented shapeshifting in certain situations), but I thought I might try to fix the problem. So, I added the following type code to the shapeshift command parsers.

if (said("shapeshift", "man"))
 {
   if (!bOnEarth)
   {
     // nCurrentEgoView is not v16

     if (nCurrentEgoView != FORM_GENERIC_MAN)
     {
       load.view(FORM_GENERIC_MAN);
       set.view(ego, FORM_GENERIC_MAN);

       if (bDiscardOldViewOnMorph)
       {
         discard.view.v(nCurrentEgoView);
       }

       nCurrentEgoView = FORM_GENERIC_MAN;
     }
     else
     {
       print (m1);
     }
   }
   else
   {
     print(m4);
   }
 }


The first time I do a morph, it works fine. If I then morph into a different form, the game gives me a view not loaded error message and quits. Obviously, the view seems to be loaded, because until two seconds ago it was assigned to ego (I checked the number of the view it said wasn't loaded and it was the correct view number for the view ego was just using).  Any ideas why this might be happening?
sonneveld A couple of things go wrong here.. basically, agi expects you to discard resources in the same order they were loaded.. since ego's view is loaded right at the start of logic.0, it gets complicated.

the thing to remember is that agi allocates memory on a sort-of stack..  It also has linked lists containing all the loaded objects (separate ones for view, pic, sound, logic), with pointers to the memory heap.  

So the resource information is contained on the heap.. and the linked list contains information on it's whereabouts in the heap and which items are loaded.

so what you have before you shapeshift is probably:

mem = GAME->LOGIC.0->EGO_VIEW->LOGIC.ROOM->SOUND->VIEW->VIEW->NEXT
logic = LOGIC.0->LOGIC.ROOM
pic = 0  // always discarding pics hopefully
view = EGO_VIEW->VIEW->VIEW
sound = SOUND

The reason why ego view is at the start is because logic.0 always loads it for a new room.  The Next is the pointer to the next position in the memory heap.

so when you load up the new view, the memory will look like this:

mem = GAME->LOGIC.0->EGO_VIEW->LOGIC.ROOM->SOUND->VIEW->VIEW->*MAN_VIEW*->NEXT
logic = LOGIC.0->LOGIC.ROOM
pic = 0  // always discarding pics hopefully
view = EGO_VIEW->VIEW->VIEW->*MAN_VIEW*
sound = SOUND

and THEN you discard the ego view.. but this is where the algorithm of the agi memory gets screwy.. When agi removes an item from the view list, it just sets the next pointer of the *PREV* item to zero... since you're removing ego_view, the first item in the list, it will remove all the others too.

view = 0

that's not all.. to remove it from memory, it just sets the "next available space" pointer to point to EGO_VIEW, so when you load a new resource, it will overwrite the old resource.. this works fine.. but if you set the next available space to ego_view, new resources will write over ALL the other resources loaded in memory..

so the memory looks like this now:

mem = GAME->LOGIC.0->*NEXT*->EGO_VIEW->LOGIC.ROOM->SOUND->VIEW->VIEW->MAN_VIEW

So most of the items are still in memory, but when you load up another resource, it will load over them, AND the view list is destroyed, so the interpreter won't know about any of the loaded views..


That's why you get a "view not loaded" and possibly other weird stuff happening...

This is definately needed for a tutorial, it's kinda complicated, but I hope I made sense.  The main thing to remember is to discard in the same order you load.. even if it means having an ego view and a man view in memory at the same time.

- Nick
Joel That actually does make sense. I think I remember hearing something like this before. It also explains why when I did a show mem the memory dropped from about 36000 bytes used to about 20000 bytes used.

I think I'm going to solve the problem by limiting the shapeshifting in all situations. I think it's better that then risking the game crashing by somebody morphing one too many times on the same screen.
sonneveld I don't think it would be much of a problem.. just store a variable that contains the ego's view resource number for when it enters a room (it will be at the start of the mem heap) and make sure your shapeshift code doesn't discard it when swapping views.. just amy other ones that you load when shape shifting in that room.

You'll have, at max, two ego views loaded (the one you started with in that room, and the current).. but it's better than limiting yourself.

- Nick
Joel ok. I did that but still ran into a problem with discarding the current ego view (it causes major problems to dispose of a view while it's associated with an object). So I created a 1x1 blank view resource and set ego's view to that, discarded the old view if it was safe, and then loaded and set the new view. It doesn't seem like it should work, but the game doesn't crash anymore and the show mem command reports the same amount of memory used every time I morph into the same form.
sonneveld The reason why you shouldn't discard the view while it's still associated with an animated object is because there's other things going on with the memory manager I forgot to mention.

There's actually a list of animated objects, with blitting information.  But because it's dynamic, and it would have to be updated all the time, whenever something it added to the memory heap, the interpreter free's all the animated objects, allocates the new bit of memory and then allocates memory for the animated objects..  That way the animated objects are always at the end of the heap.

can't you keep another temp variable storing what the last view resource was?  then discard it after you're set it?

Another thing you could do, is in logic.0.. when it loads up the ego view resource.. you could add a check so it doesn't load it in the rooms where jen shapeshifts.. then in the room logic, you load up the ego as the last resource.. that way you can discard it at your leisure.

- Nick
Joel I guess what is going on is this:

views->EGO_VIEW->VIEW->VIEW

when a shapeshift command is executed the first time

views->EGO_VIEW->VIEW->VIEW->NULL_VIEW->NEW_EGO_VIEW

and because the previous view is the view on room entry, no discard command is executed. On the second shapeshift command, it sets ego's view to NULL_VIEW (which is already loaded -- I guess the interpreter's smart enough not to load it twice), then it deletes NEW_EGO_VIEW and loads the next view in as NEW_EGO_VIEW. Unfortunately, this means I'll have to be very careful about loading new views in rooms where unrestrained shapeshifting is allowed., but I should be able to handle it.
Joel I do keep a variable that says what the last view of ego was. But I think if I set ego's new view first and then discard the old, the same problem will happen where the resource linked list loses its reference to the newly loaded resource.
sonneveld what I meant was not load the ego view until last..  

so in logic.0.. you stop it with an if statement.. after you've loaded all the other views, you load up the ego's view..
so the view list will look like:

view = VIEW->VIEW->EGO_VIEW

then you can safely discard the ego_view.. as long as you didn't load a sound or a picture after it.

- Nick
Joel ok, I'm talking about something a little different. I mean when the view being discarded is not the initial ego view at the start of the room. I use what I've called the null view because I think if I don't then I'll get this situation

view = ORIGINAL_EGO_VIEW -> ... -> CURRENT_EGO_VIEW -> TARGET_EGO_VIEW

after I set ego's view to TARGET_EGO_VIEW, if I discard CURRENT_EGO_VIEW, won't that also discard (i.e., lose the memory reference to) TARGET_EGO_VIEW? It sets CURRENT_EGO_VIEW.prev.next to NULL?

I don't think keeping ORIGINAL_EGO_VIEW in memory eats up enough resources for it to be worth making serious modifications to the behavior already written into logic 0. In most situations where memory is tight, unrestrained morphing isn't allowed anyway. I'll keep that idea in mind in case I find out that it is a problem later on, but for now I think it's ok to just have two ego view resources in memory at once.
sonneveld Sorry, I understand now.. you're trying to avoid having two views for the ego loaded.. even if you're just swapping them.

I think the best way would be the method you're using with the 1x 1 view..

my idea was to defer loading the ego view until later.. when you've loaded up the other resources.. that way you could safely discard it when swapping..

btw.. the interpeter's smart enough not to try and load resources twice, but it will still add an entry to the game script, despite the fact it's already loaded.  So eventually the script will fill up in that room.

- Nick
Joel Ok. Two load.view commands get executed every time a morph command is executed. I know this depends on how much other stuff is loaded, but as a rough guess how many times would someone have to shapeshift in a single room to fill up the script buffer? Would it be a lot?

If so, I think I could probably leave checking for that kind of thing out, because there won't be that many reasons to morph in places where unrestrained morphing is actually allowed. If it's not a lot, I could put an upper limit on the number of morphs allowed in a single room or within a certain amount of time or something like that.
sonneveld I can't remember what the default maximum is.. but you can set your own script size (right at the start!  before any menus, logics or anything else is loaded.. and only once).

mem.info also gives you the max script that was reached in the game.

There's a flag that disables writing to the script.. and two unknown functions save, restore the position in the script.

if you disable the script whilst morphing.. you'll have problems with saving/restoring though (because that's what the script's for!)..

so, before you save, .. disable script write, discard the ego view resource, enable script again and load the resource.

that way, when the restore game occurs, it will read the script.. load the last view resource, and then, when it assigns view resources to animated objects.. it will have a view loaded to associate with it.

i hope that made sense.

- Nick
Joel The default size is 50, and it takes probably somewhere between 15 and 25 morphs to overflow the buffer.

Ok, I'm not 100% clear what you're saying. Let me say it back to you to see if I understand. What you're saying is that I should write something like this:


 if (said("shapeshift", "man"))
 {
   if (!bOnEarth)
   {
     if (nCurrentEgoView != FORM_GENERIC_MAN)
     {
       // disable script buffer
       set(script_buffer_blocked);

       load.view(FORM_NULL);
       set.view(ego, FORM_NULL);

       if (bDiscardOldViewOnMorph &&
           nCurrentEgoView != nEgoViewOnRoomEntry)
       {
         discard.view.v(nCurrentEgoView);
       }

       load.view(FORM_GENERIC_MAN);
       set.view(ego, FORM_GENERIC_MAN);

       nCurrentEgoView = FORM_GENERIC_MAN;

       // reenable script buffer
       reset(script_buffer_blocked);
     }
     else
     {
       print (m1);
     }
   }
   else
   {
     print(m4);
   }
 }


then when a game is saved I should say something like

if (bDiscardOldViewOnMorph &&
   nCurrentEgoView != nEgoViewOnRoomEntry)
{
   set(script_buffer_blocked);
   load.view(FORM_NULL);
   set.view(ego, FORM_NULL);

   discard.view.v(nCurrentEgoView);

   reset(script_buffer_blocked);

   load.view.v(nCurrentEgoView);
   set.view.v(ego, nCurrentEgoView);
}

// now save the game here



is that what you're saying?
sonneveld I haven't tested it myself.. but that's the idea.

I'd actually be interested if it does work.. because it was just a theory I had in my head and if it works, it would help lots of people who want to continuously load random resources in a single room.

ie, a jukebox that plays different music but there's not enough room to load all sounds at once.

- Nick
Joel Ok, I implemented it, and I seem to be able to save and restore a game ok. However, if I restore a game and then try to shapeshift, unpredictable stuff happens. Once, the screen got garbled. Another time, Windows said the program did something illegal and I should restart my computer. Another time, the game just locked. However, using NAGI I saved and then restored and nothing bad happened.
sonneveld NAGI's had to work around a few bugs that don't get found as easily in the dos agi. :)  grr kings quest 4...

Can you trace through the code?  Make sure tracing is turned on (so scrolllock works), and then call trace.on() from inside the shapeshift code?

It might take a few goes but I'd like to know if the weirdness occurs during the logic execution or just after when it does all the graphics and starts logic.0 again.  It'll give me an idea of what to look for in the agi "source".

btw.. nice to see that you've registered. :)

- Nick
sonneveld also, call show.mem before and after the discards and make sure the difference in free heap is around the size of the view (they'll be some small differences because of linked list structures as well.)

- Nick
sonneveld I'm not quite sure where the error's happening yet.. but also, make sure if you're setting a loop for the view, it's set to a number within the range.

Also, i was wrong around script filling up.  it will only add to the script if the resource isn't loaded.  (ie, if you discard and then reload.. then the script gets filled).  that doesn't help if you're discarding anyway.

- Nick
Joel The problem is definitely happening in the morph handler. I can't get it to print out command names for some reason, but the last few lines before the game either goes weird or Windows talks about an illegal operation are (my guesses as to what the corresponding code is are beside them):

41(0,33) - set.view(ego, FORM_NULL)
7(27)     - isset(bDiscardOldViewOnMorph)
2(37,39) - bCurrentEgoView != nEgoViewOnRoomEntry
 (28,9)  - no idea
153(37)  - discard.view.v(nCurrentEgoView);
    (28)  - no idea
31(40,3) - load.view.v(nTargetEgoView)
(press Enter here it screws up)


if (nTargetEgoView != FORM_NULL)
{
 if (!bOnEarth)
 {
   if (nCurrentEgoView != nTargetEgoView)
   {
     set(bScriptBufferBlocked);
     load.view(FORM_NULL);
     set.view(ego, FORM_NULL);

      if (bDiscardOldViewOnMorph &&
          nCurrentEgoView != nEgoViewOnRoomEntry
     {
       discard.view.v(nCurrentEgoView);
     }

     load.view.v(nTargetEgoView);
     set.view.v(ego, nTargetEgoView);

     nCurrentEgoView = nTargetEgoView;

     reset(bScriptBufferBlocked);
   }
   else
   {
     print(m1);
   }
 }
 else
 {
   print(m4);
 }

 // reset the target view to NULL so this handler
 // doesn't execute again
 nTargetEgoView = FORM_NULL;
}
sonneveld it's a bit late, I'll have to look closer tomorrow.. but is there a chance you'd be able to email the game?  even if it's just the room with the morphing code?

I'll be able to go through it with a fine tooth comb then..

- Nick
df how well does it work under sarien, if at all ;) since sarien would definatly use different memory handling that old agi ;) i'd assume nagi does it differently as well...
AGI1122 It seems to me that NAGI is more lenient memory wise than Sarien. When V still had those memory problems the original interpreter would not run it at all, and Sarien would work for some of the intro but crashed on the skyscraper scene with the 2 ships. NAGI was the only one that was able to run it then. But thanks to Nick the memory problems are gone and V will run on all the interpreters including the original.
sonneveld I don't think it was a problem with Sarien's memory handling.  Sarien doesn't even use the load/discard commands.. it just loads whenever it needs to.  I'd say it probably was a problem with sarien's sprite handling in that V room.

- Nick
sonneveld I think it's a problem with the list of animated objects.. somewhere it's getting corrupted and crashes interpeter when drawing them on screen... investigating..

- Nick
sonneveld ok, found the problem..

it was still the old "load something and then discard something else" bug again.  It was staring me right in the face.. but I went ahead and used debug, printf statements 'n everything.  Actually, after going through the nagi printf statements slowly, I found out where the bug was.

here's the code (there's similar code you had for the save game wrapper as well):

load.view(FORM_NULL);
set.view(ego, FORM_NULL);

 if (bDiscardOldViewOnMorph &&
     nCurrentEgoView != nEgoViewOnRoomEntry
{
  discard.view.v(nCurrentEgoView);
}


so basically, you're loading the null view so you can discard the other view.. which means you're technically discarding the null view resource too. (because it's next on the list.. after the current view resource that's about to be discarded).

so what I did:

1) in the new.room section of logic.0, I load the null view resource there.  that way it's always available if you want to discard another view object
2) remove loading the null view resource in logic.0 for saving and in logic.90 for morphing..  It still sets the ego to null view though.. just doesn't load it again.

and it works.  I've quickly tested it, saving and restoring and there's no problems.

you might want to test a bit more to see if memory or script runs out but I think it's working fine.  I did a quick test with "show mem" and the max script level didn't rise.

So I hope that ends the chapter of AGI Memory Management Problem. :)

- Nick
sonneveld btw.. I love it how you can have Jen walking, but still morph while she's walking. :)

- Nick
Joel It seems to be working for me, too. Thanks for looking at it and for all your help.

I do have a couple more minor questions to make sure I don't run into more errors down the road. First, I know this is something I need to be concerned with any time I load a new view, but is it also something I need to be concerned with any time I load ANY new resource? As in, do I need to discard ego's view and reload it if I load a new sound into memory after it has been possible for the player to morph, for example?

Second thing is, when morphing into a view that is smaller than the current ego view (most notably morphing from just about anybody into the fairy from King's Quest), there's occasionally residue left on the screen from the previous view that doesn't go away until something is moved over it on the screen (for example, ego walks over it or a print box covers it up). Any ideas what might cause that? It may be a non-issue, because I don't think I'm going to leave the ability to morph into Sierra characters in the game, anyway. I'm just curious, 'cause it's a little ugly to have the previous view's head floating on the screen after a morph.
AGI1122 What I usually do to avoid the character from being left on the screen like that when changing the view is:

erase(ego);
set.view(ego,2);
draw(ego);

this will get rid of that ugly problem with the character being left on the screen.
sonneveld AGISCI's suggestion sounds like the best solution for changing view's.

If you load anything else, you'll have to discard and then reload the view resource too.  It's fiddly so you'll probably have to make sure that you load up all sounds and stuff beforehand.

You could always generalise it, you could have a separate logic function that checks a list of animated objects and their view resources.

If you plan on loading another resource, you call this separate logic to give those specific animated objects (only "morphing ones") a null view, unload all the view resources..  

then open your sound resource

and then call this logic again to load up the views and give them to the animated object.

I dunno how feasible it would be, but you could call it anywhere.

- Nick