Dynamic sprite issue: when assigning sprite may not update character looks

[Crimson Wizard]

Well, damn, looks like I just run into one of those obscure AGS bugs. Took me several hours to, first, realize that could not be a mistake in my script, and then find out why is it happening exactly, staring into the engine code.

Following example is based on default game template.
Room 1 script:

Code: ags

// room script file

DynamicSprite *Sprite;

function room_Load()
{
  Sprite = DynamicSprite.CreateFromExistingSprite(2000);
  ViewFrame *f = Game.GetViewFrame(player.View, 0, 0);
  f.Graphic = Sprite.Graphic;
}

function on_key_press(eKeyCode key)
{
  // These lines cause the bug ======
  //if (Sprite != null)
  //Sprite.Delete();
  // ================================
  
  DynamicSprite *new_sprite = DynamicSprite.CreateFromExistingSprite(2000 + Random(9));
  Sprite = new_sprite;
  ViewFrame *f = Game.GetViewFrame(player.View, 0, 0);
  f.Graphic = Sprite.Graphic;
}

So, this code is very simple. What it does:
- It takes certain View frame, and replaces original sprite with dynamic sprite, which in turn is a copy of some character sprite.
- On every key press it creates NEW dynamic sprite from the random range of character sprites, and puts it to same View frame.
Don't bother wondering why would I need such script, that's just an example of certain actions.

Run the game and hold any key (e.g. space bar). Character's image will beging to cycle randomly.

Now, the interesting part. See these commented lines where the previous sprite gets explicitly DELETED before assigning new sprite to the same pointer? Thinking logically, that should be right thing to do, and should be safe to assume that nothing bad is going to happen?

Well, uncomment these lines:

Code: ags

if (Sprite != null)
    Sprite.Delete();

and run the game again. Hold space bar, and... you see that character's looks do not change anymore.
HOWEVER, if you walk a little and then make Roger take original direction, you will see, that the frame 0 sprite actually is different. So it DOES change after all, it's just that AGS did not redraw it in time.


So, why is this happening, and how can DynamicSprite.Delete make any difference?


AGS has character cache, where it stores last character's image with additional effects (tint, lighting, area zoom) applied. On every update it checkes whether anything has changed and if not - it keeps cached sprite.
When you create NEW dynamic sprite, and assign it to the view frame, the number of sprite is now different from old one, so AGS knows that it needs to reset cache.

But when you delete an old sprite FIRST, its SLOT number frees, and the next dynamic sprite gets created on the SAME SLOT, having same index as the old one has had! And AGS cannot detect that the change happened!

To prove this point, change the code:

Code: ags

function on_key_press(eKeyCode key)
{
  // Temporarily keep an old sprite in another pointer
  DynamicSprite *old_sprite = Sprite;
  
  DynamicSprite *new_sprite = DynamicSprite.CreateFromExistingSprite(2000 + Random(9));
  Sprite = new_sprite;
  ViewFrame *f = Game.GetViewFrame(player.View, player.Loop, player.Frame);
  f.Graphic = Sprite.Graphic;
  
  // Now delete the old sprite by referencing the temp pointer
  if (old_sprite != null)
    old_sprite.Delete();
}

If you run this code and hold space bar, the character will be animating again.


Now, the above is just a dummy script example. If you have a more complicated script, with multitude of dynamic sprites, where you cannot reliably keep track of when the old slots are freed (or rather do not want to do that), for that case I found another workaround to force AGS reset character cache:

Code: ags

DrawingSurface *ds = Sprite.GetDrawingSurface();
ds.DrawPixel(-1, -1);
ds.Release();

Thing is that when DrawinSurface.Release is called, it checks all (supposedly) places where this sprite could be used, and releases all related caches. But some drawing operations MUST be complete to make it do so! Hence we just draw one pixel outside of the sprite. While sprite physically stays unchanged, DrawingSurface object registers modification.

Now this will work too:

Code: ags

function on_key_press(eKeyCode key)
{
  if (Sprite != null)
    Sprite.Delete();
  
  DynamicSprite *new_sprite = DynamicSprite.CreateFromExistingSprite(2000 + Random(9));
  Sprite = new_sprite;
  ViewFrame *f = Game.GetViewFrame(player.View, player.Loop, player.Frame);
  f.Graphic = Sprite.Graphic;
  
  // Poke AGS to make it reset sprite cache
  DrawingSurface *ds = Sprite.GetDrawingSurface();
  ds.DrawPixel(-1, -1);
  ds.Release();
}

PS.
The actual reason for this bug is that when DynamicSprite gets deleted, it does not clears related CHARACTER caches. Which is very strange, because it clears GUIs and Room object caches. This makes me think that there could be a simple oversight.


PPS.
Also, that bothers me, but I have a suspicion that if you don't call DynamicSprite.Delete, but simply overwrite pointer with new sprite, the actual bitmap does not get deleted and stays in system memory (aka "memory leak"). This is explicitly coded so in AGS engine, and I cannot understand why. This might require a separate research.

[Khris]

The manual says that .Delete() is supposed to be used with sprites that are created from external files because those aren't deleted automatically.

[Crimson Wizard]

The manual says that .Delete() is supposed to be used with sprites that are created from external files because those aren't deleted automatically.

I guess this text might be remaining from old times when there were no managed classes (OO-style script), but functions like LoadImageFile and DeleteSprite. Because with DynamicSprite class there is no difference in where the sprite came from, external file, or created with DynamicSprite.Create, and so on.
For example, DynamicSprite.Create article also mentions that it has to be deleted:

IMPORTANT: This command loads an extra sprite into memory which is not controlled by the normal AGS sprite cache and will not be automatically disposed of. Therefore, when you are finished with the image you MUST call Delete on it to free its memory.

EDIT: please ignore the rest of the post , I found out I am made wrong assumption about memory leaks:

On other hand, it also sais:

IMPORTANT: If the DynamicSprite instance is released from memory (ie. there is no longer a DynamicSprite* variable pointing to it), then the sprite will also be removed from memory. Make sure that you keep a global variable pointer to the sprite until you are finished with it, and at that point call Delete.

But I am not so sure if that's true, because from the looks of the engine code it does not do that, hence the difference in behavior, which I described in the first post.
BTW, I find quoted phrase contradictory; if the sprite is going to be released after pointer is nullified, why the need to call Delete at all? In practice it seems that you must call Delete() anyway to avoid memory leaks.

My point is though, that since there is a managed memory system now, it is still supposed to clean up all resources after an object has its reference count equal to zero and is disposed. At least if that resource is not referenced by anything else.
Maybe the actual bitmap is not deleted like that because Chris Jones tried to keep support of old functions this way... not sure if this makes sense though.

[Crimson Wizard]

I am sorry, I made a mistake when reading the code, and basically understood some condition opposite to what it really meant.
There is no memory leaks, it actually does what it should - deletes the actual bitmap as soon as script object's reference count is zero.

This means that it is not necessary to call Delete function if you have only one DynamicSprite pointer and nullifying it. Perhaps it may be still useful if you have number of pointers referencing same object, but want to delete the bitmap without nullifying every variable.



UPD:

In case I caused confusion, it was not really a call to DynamicSprite.Delete that causes the issue. In the script examples posted above one could replace "Sprite.Delete();" with "Sprite = null;" and get essentially same results.

The cause of the issue is:
1) existing dynamic sprite gets deleted, and its slot (index) freed.
2) new dynamic sprite is created, and gets to the same slot, because it's now the first free slot in the sprite cache.
3) the character's look does not get updated on screen, because its view frame points to sprite's INDEX rather than memory object, and since the index remains the same, AGS cannot detect the change.

This is a bug in the engine, that probably can be fixed by clearing an entry in character's view cache when the dynamic sprite is deleted.