Best way to delete an instance of a game object

[Tomi]

"What I like about programming is, there is always more than 1 way to do the same thing."

I agree with you, Handoko. In programming there are many paths to the solution.

My game is an RTS and the main problem with it that I check the existence of gameunits not only in the main loop, but e.g. in FormPaint too, where I draw their picture. And in this case I get an error message and I don't know, why?
This is the code of checking everywhere in my program:

Code: Pascal  [Select][+][-]

1. if numberofunits>0 then
2.  begin
3.    for i:=0 to numberofunits-1 do
4.    begin
5.       if not gameunits[i].exists then (...)

Maybe I'm organizing my program badly.

P.S: I just replaced this:

Code: Pascal  [Select][+][-]

1. if gameunits[i].exists

to this in my whole program:

Code: Pascal  [Select][+][-]

1. if gameunits[i]<>nil

and it seems the game works without errors. But I don't see why this is better? Just because I remove unnecessary instances this way:

Code: Pascal  [Select][+][-]

1. freeandnil(gameunits[i]);
2. dec(numofunits,1);
3. setlength(gameunits,numofunits);

So after this remain some units? FreeAndNil doesn't remove immediately those instances?

[Handoko]

Badly organized code may still run correctly. But well organized code will make maintaining the code easier.

Try this:

Code: Pascal  [Select][+][-]

1. // Draw objects to screen
2. for i := 0 to Length(GameUnits)-1 do
3. begin
4.   if not(GameUnits[i].Exist then continue; // skip drawing it
5.   case GameUnit[i].ObjectType of
6.     otTree: begin
7.       // Draw tree
8.       end;
9.     otEnemy1: begin
10.       // Draw enemy model1
11.       end;
12.     otEnemy2: begin
13.        // Draw enemy model2
14.       end;
15.     otPlayer: begin
16.        // Draw the play
17.       end;
18.     // ...
19.   end;
20. end;
21.  
22. // Remove all unused objects
23. DoRemoveUnused;

I think you should not perform drawing and removing objects at the same time. Painting objects to the screen (especially when using TCanvas) requires time. You can remove the unused objects after or before the drawing.

If you ever write games, you may ever experience screen flickering. Unless you're using game/graphics specialized library,
the time period for updating screen should be kept as short as possible, it is necessary to prevent flickering. That means you should perform once for drawing all the objects and you shouldn't doing any heavy calculations while updating the screen, which including freeing objects.

You can learn more about screen flickering here:
https://forum.lazarus.freepascal.org/index.php/topic,38136.msg263143.html#msg263143

If you share the whole source code here, I may able to help you fix the bug. If you're not willing to publicize the code, you can email it to me.

[Tomi]

Thanks, Handoko!
And what is your opinion about FreeAndNil? Doesn't it delete immediately instances? (See the postscript in my previous post.)

[Handoko]

The bug should be in somewhere else. If you correctly marked unused object by setting GameUnits[?].Exist := False; and properly removed them. Your previous code should work without issue.

Now, I see a potential bug:

Code: Pascal  [Select][+][-]

1. freeandnil(gameunits[i]);
2. dec(numofunits,1);
3. setlength(gameunits,numofunits);

So after this remain some units? FreeAndNil doesn't remove immediately those instances?

An error may happen if, for example:
NumOfUnits = 10
i = 5

Because FreeAndNil(GameUnits[5]) will cause item #5 to be removed from memory. But SetLength(GameUnits, NumOfUnits) will remove item #10 from the array.

[Tomi]

"An error may happen if, for example:
NumOfUnits = 10
i = 5"

A-ha! But what is the solution? Should I use Delete(gameunits,i,1); instead of FreeAndNil()? Or instead of SetLength()?

[Handoko]

Nothing wrong with FreeAndNil. You just need to rearrange the array correctly before calling SetLength.

That can be done by this pseudo code:

Code: Pascal  [Select][+][-]

1. TargetIndex := 5;
2. If TargetIndex < ItemCount then
3.   for i := TargetIndex to ItemCount-1 do
4.     Items[i] := Items[i+1];
5. SetLength(Items, ItemCount-1);

[Tomi]

Thank you very much, Handoko! I hope, this solution will be good.