['Possible' GLITCH] with Dynamic array of DynamicSprite

[Ryan Timothy B]

I have a dynamic array in DynamicSprite format.  This array is created at game start, like so (this example only has 1 for the array size, but the issue occurs with any amount):

Code: ags

DynamicSprite *test[];

function game_start() 
{
  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);
}

Then I press F9 to restart the game, which recreates the array.  Once I close the game, I get the following error:

An error has occurred. Please contact the game author for support, as this is likely to be a scripting error and not a bug in AGS.
(ACI version 3.20.1099)

Error: DeleteSprite: Attempted to free static sprite X that was not loaded by the script

(X being the sprite number it stored it in; which is irrelevant)

[Ryan Timothy B]

Ahh, there we go.  I know how to prevent it from glitching.

Code: ags

function on_key_press(eKeyCode keycode) 
{
  if (keycode == eKeyF9) 
  {
    int i;
    while (i<1)
    {
      if (test[i]!=null) test[i].Delete();
      i++;
    }
    RestartGame(); // F9
  }
}

[Monsieur OUXX]

Correct me if I'm wrong, but does the issue also happen in that scenario :

Code: ags

  DynamicSprite *test;
  test = //any non-null value
  test = //any non-null value

It would mean that you simply can't affect a value to a variable of type  DynamicSprite * twice in a row. It has to be null.



EDIT: Hey wait a minute. did you edit your very first post? If not, then I apologize, I completely misread it.

[Gilbert]

I've never tried, but I think this could still work.
However, from what I have read in the manual, it seems that a DynamicSprite won't be destroyed automatically when no pointer is pointing at it any more (unlike Overlays, which are destroyed once there is no pointer pointing to it). This would make DynamicSprites a bit hard to use, as you have to call Delete() first whenever you need to assign a pointer to another new sprite (more troublesome is that if the pointer is null calling Delete() will crash the engine, so you have to inconveniently check the pointer whether it is null before destroying the sprite). Careless handling of DynamicSprites (i.e. forget to call Delete() before assigning a pointer to a new sprite, or declare a pointer within a function and have the pointer destroyed after the function ends) can cause memory leak, so I'd expect the above codes would work as intended but would also cause a memory leak at the same time.

[Monsieur OUXX]

I'd expect the above codes would work as intended but would also cause a memory leak at the same time.

Yes, that's for sure. I was trying to reproduce Timothy's scenario, but to remove all unnecessary/misleading stuff.

[Ryan Timothy B]

However, from what I have read in the manual, it seems that a DynamicSprite won't be destroyed automatically when no pointer is pointing at it any more

so I'd expect the above codes would work as intended but would also cause a memory leak at the same time.

But the manual itself states:

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.

So with Monsieur's code, the second time it assigns the non-null value to the sprite, it removes the pointer from the original sprite and then AGS automatically deletes it.
I even tested it with:

Code: ags

  int i;
  while (i<30000) 
  {
    test = DynamicSprite.CreateFromExistingSprite(0);
    i++;
  }

And the memory usage remains the same as without the while loop (although it takes forever to finish the loop, heh).


I think it's just an overlooked issue with Dynamic arrays that CJ didn't think of.

[Monsieur OUXX]

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.

It's indeed tricky. Just above, it says :

[the dynamic sprite] 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.

What you must understand is :
- As soon as no variable points to the Sprite, then it becomes "unusable". You must forget about it (it's like it'd already be deleted)
- BUT it's still stored in memory. You must Delete it to avoid unnecessary use of memory.


That's exactly what happens in the scenario you described :
1/ You create a dynamic array. One cell of the array is like a variable pointing to a dynamic sprite.
2/ You restart the game. The array is deleted. You lose the variable pointing to the sprite. But the sprite is still in memory.
3/ When you close the game, it does its standard inspection to see if there are still undeleted sprites in memeory. Seeing that there is one, it complains and calls you a bad programer.

[Gilbert]

But the manual itself states:
(bla bla)

Note this from the entry of Create():

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.

I'll say the manual is quite confusing. I actually am not sure whether a DynamicSprite is destroyed automatically when no pointer is pointing to it, but from my interpretation of this paragraph it is a 'no'.

Edit: Okay, Monsieur beats me to it and I didn't read it.

[Ryan Timothy B]

From both your responses I'm still at a loss to understand if you're saying I'm a bad programmer who isn't deleting his dynamicSprites
OR
It's an issue with AGS.

Because even doing this:

Code: ags

  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);
  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);

back to back will not cause an issue, nor will the memory usage change from this way to the other.
The dynamic array is still deleting itself, but at least this way it actually deletes the DynamicSprite on its own but just not when the game restarts.

[Gilbert]

Why? I have mentioned I am not familiar with this and am not sure about the real behaviour (and I'm just too lazy to find out), but from my interpretation of the manual it's not automatically deleted (unless it has other cryptic meanings).

I just have thought about it, maybe one reason for this is that sprites can also be referenced by numbers. If say you assign a DynamicSprite to say an object or a frame in a loop (by using DynamicSprite.Graphic) and if the sprites are deleted when the pointers are destroyed then the game can crash, though it's really a bad habit to still let the game take reference to a sprite by number if there isn't any pointer to it anymore, as this would mean the sprites can never be deleted.

To test the real behaviour, one suggestion is to save the sprite number of the created sprite to a variable, and then destroy the pointer (either calling CreateBlah() again or setting the pointer to null). Then try to display that sprite using the saved number. If the game does not crash and the sprite can still be accessed, that means DynamicSprites indeed won't be deleted automatically and then we need to take extreme care that we have top Delete() them every time we have to dispose them.

[Monsieur OUXX]

Hi Timothy,

I believe I see where the misunderstanding comes from, but instead of telling you "that's the way it is", I'd rather make it clear for you.

Let's start again from a simple script and let's make tests.

TEST 1:
Run your script without creating ANY dynamic sprite, and note down the memory usage.

TEST 2:

Code: ags

  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);

For that test I'd like you to use a sprite as large as possible, so that you can clearly see the space used by the sprite. Compare it with the space used by TEST1.
Space(TEST2) - space(TEST1) = sprite size.


TEST 3:

Code: ags

  test = new DynamicSprite[2];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);
  test[1] = DynamicSprite.CreateFromExistingSprite(0);

Compare that with the space used by TEST1 and TEST2. You should see the difference caused by test[1].
You should see:
Space(TEST3) = space(TEST2) + sprite size = space(TEST1) + 2*sprite size

========

Are we OK?
Now, for your specific situation :

TEST 4:

Code: ags

  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);
  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);

I believe you should see approx. the same space usage as in TEST 3.
You should see:
Space(TEST3) = space(TEST1) + 2*sprite size

=======

Explanations :


The first time you do :                 test = new DynamicSprite[1];
...you create an array.
When you do :                              test[0] = DynamicSprite.CreateFromExistingSprite(0);
...you do 2 things at the same time:
    1/ you create a dynamic sprite
    2/ you save a pointer to that sprite in a cell of the array.


The second time you do :            test = new DynamicSprite[1];
...you do many things at the same time:
    1/ You lose the pointert to the existing array. Since arrays are managed automatically, you delete the array "test"
    2/ since you've lost the array, you also lose the pointer to the Dynamic sprite. Dynaic spr are not managed dynamically, so, even though you've lost the pointer to it, it's NOT deleted. You can't use it anymore but it's still floating around in memory.
     3/ You create a new array
When you do :                              test[0] = DynamicSprite.CreateFromExistingSprite(0);
     4/ You make one of its cells point to the second dynamic sprite you've just created.


When you restart the game :
   1/ the array is automatically destroyed
   2/ any pointer stored in it is lost
   3/ the dynamic sprite referenced by the lost pointer is not deleted (and even if you delete that one yourself, there's still the first dynamic sprite floating around)
   4/ the engine complains that one or several dynamic sprites haven't been deleted.

=======

Now, for your questions :

Because even doing this back to back will not cause an issue.

Well it should, at least when you restart the game, because, as I've demonstrated, you still don't delete one of the sprites.

Because even doing this back to back will not change the memory usage. It actually deletes the DynamicSprite on its own but just not when the game restarts.

I believe you're mistaken. TEST4 uses the same space as TEST3. The first dynamic sprite is never deleted, since you lose the pointer to it by destroying your array.





The dynamic array is still deleting itself, but at least this way it actually deletes the DynamicSprite on its own but just not when the game restarts.
[/quote]

[GarageGothic]

Wait, doesn't restarting the game just restore an autosave (slot 999 if I recall correctly)? So that would mean restoring normal savegames would also cause this to happen? 

[Monsieur OUXX]

Well in any case there's one thing that's for sure :

ALWAYS DELETE THE DAMN SPRITE!

[Gilbert]

All right. I just have a little test with my idea as posted above.

Test 1
In Enters Room after Fade-in of a room, create a DynamicSprite from an existing sprite, save the sprite number to an integer and then set the pointer immediately to null. Finally, create a graphical overlay using this sprite number.
Result: The game crashes (it won't crash if the pointer is not set to null), so it seems that when you destroy a pointer by setting it to null the engine would destroy the sprite accordingly (memory usage not checked though, so it may still be possible that there is a memory leak).

Test 2
Continue with what has been done in Test 1, but without setting the pointer to null, add some codes in on_key_press() so that it will create a graphical overlay using that saved sprite number, with the sprite number saved in a variable declared outside of functions, and the DynamicSprite pointer declared inside the Fade-in event (so that point is completely destroyed after the function ends).
Result: The game crashes again, as expected.

So, far it seems that the engine is able to dispose of orphaned DynamicSprites automatically. HOWEVER:

Test 3
Continue with what has been done in the previous two tests, but instead of setting the pointer to null, it's now destroyed by creating another DynamicSprite with another existing sprite and have the pointer assigned to it (and with this pointer declared outside of functions to keep it alive).
Result: The game does not crash, either when you then make the overlay with the sprite number immediately, or make an overlay later when pressing the spacebar. However, the sprite is displayed as the legendary blue cup instead of the originally specified sprite. So, it seems that the DynamicSprite is sort of destroyed, but somehow the sprite number is still valid, unlike in the previous two tests.

So, the results are not very consistent and for safety's sake, always Delete() unused sprites for the mean time, until CJ can explain on the actual behaviour of the matter.

[Pumaman]

An error has occurred. Please contact the game author for support, as this is likely to be a scripting error and not a bug in AGS.
(ACI version 3.20.1099)

Error: DeleteSprite: Attempted to free static sprite X that was not loaded by the script

This looks like a bug in AGS, probably related to restoring a save game if you're using a dynamic array of dynamic sprites. I'll see if I can reproduce it.

[Ryan Timothy B]

First off M. Ouxx, I love how you're trying to explain to me how a dynamic array works.  When all I've been trying to tell you that your assumption of how AGS 'doesn't' delete sprites in a dynamic array when you recreate it is wrong.

TEST 4:

Code: ags

  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);
  test = new DynamicSprite[1];
  test[0] = DynamicSprite.CreateFromExistingSprite(0);

I believe you should see approx. the same space usage as in TEST 3.
You should see:
Space(TEST3) = space(TEST1) + 2*sprite size

Like I've mentioned in the post above, this won't store 2 instances of that sprite, with one hidden unusable in the ram.  I've tested it again and again with a very large sprite size, checking the memory usage in the task manager and it remains identical for each instance.  Unless for some reason the task manager is unreliable.



The only thing with DynamicSprites that I'm unfamiliar with, and apparently you two and probably everyone else.  When is it that you MUST delete a DynamicSprite?  Do they actually need to be deleted before you restart the game, or quit it?

I would love feedback on this one CJ. 
I was playing VinceTwelve's InfinityBit the other day and when I quit his game it created a text file with a warning of a few DynamicSprites that weren't deleted.  I on the other hand have never seen this text file with one of my games in production and I've used DynamicSprites for quite some time now.

[Gilbert]

Did you ever read my tests? The behaviour of DynamicSprites is a bit confusing. It seems that memory is indeed freed automatically, BUT in some cases the sprite numbers are still valid, so worst case scenario is that if you don't delete sprites when you dispose them things might get corrupted (or maybe, nothing fatal would happen at all), so unless there is some clarification it's still recommended to have them deleted first.

Otherwise, I'm not quite sure why there is a Delete() in the first place, as setting a pointer to null seems to get rid of the sprite anyway. (All right, maybe, it is introduced as a measure so that the sprite is guaranteed to be deleted, in case say, two or more pointers are pointing to the same spite, and so destroying just one of them won't get the sprite removed.)

[Monsieur OUXX]

Did you ever read my tests? The behaviour of DynamicSprites is a bit confusing. It seems that memory is indeed freed automatically, BUT in some cases the sprite numbers are still valid

Yes, it seems that there is indeed a glitch. Sorry Timothy, I've been trying to debunk it rationally by offering practical tests (that would separate the "array" aspect and the "DynamicSprite" aspect) - I was certainly not trying to prove you wrong.
And, as you demonstrated, the tests don't respond as expected. Well done!
Gilbet's test was also crucial.

Now it seems all we have to do is to wait for CJ's return on reproducing the issue.

[Ryan Timothy B]

Gilbet, yes I definitely did read your tests and they were very interesting.

[Calin Leafshade]

Sorry to bump gentlemen but i've just come across this issue in 3.2.1 so i thought I would mention that the issue is not yet fixed so that CJ (or another developer OMFG) can take a look.