Problems to free an Object

[krull]

Hello,

I think this is the last time, I need help with this program. It should become a vertical shoot 'em up.

The problem is that everytime if you take the 1UP('heart') or if it leaves screen bounderies, you get an Error back. Here the procedures for 'heart' and the Invocation of the Object.
Thank for your Help.
 

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

1. procedure TForm1.EnemyControlTimer(Sender: TObject);
2. begin
3.   randomize;
4.   if Enemy1.FreeMe=true then
5.   begin
6.     if not (Assigned (heart1)) {and (random(8)=8)} then
7.     begin
8.       heart1:=THeart.create();
9.       heart1.pHeart:=Enemy1.pEnemy1;
10.     end;
11.     Enemy1.destroy();
12.     Enemy1:=TEnemy1.create(1);
13.   end;
14.   if Enemy2.FreeMe=true then
15.   begin
16.     if not (Assigned (heart1)) {and (random(8)=8)} then
17.     begin
18.       heart1:=THeart.create();
19.       heart1.pHeart:=Enemy2.pEnemy1;
20.     end;
21.     Enemy2.destroy();
22.     Enemy2:=TEnemy1.create(2);
23.   end;
24.   if Player1.FreeMe=true then
25.   begin
26.     Player1.destroy(); sleep (500); halt(0)
27.   end;
28.   if Assigned(heart1) and Heart1.FreeMe=true then
29.   begin
30.     FreeAndNil(heart1);
31.   end;
32. end;                
33.  

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

1. constructor THeart.create();
2. begin
3.   heart:=TBGRABitmap.Create('thunderflash.png');
4.   heartSpeed:= 8;
5.   FreeME:=false;
6.   heartPosTimer := TTimer.Create(nil);
7.   heartPosTimer.Interval := 25;
8.   heartPosTimer.OnTimer := @heartPosTimerTimer;
9.   heartPosTimer.Enabled := true;
10. end;
11.  
12. destructor THeart.destroy();
13. begin
14.  
15. end;
16.  
17. procedure THeart.heartPaint(p:Point;pOld:Point);
18. begin
19.   Form1.Canvas.FillRect(pHeartFormer.x,pHeartFormer.y,pHeartFormer.x+heart.Width,pHeartFormer.y+heart.Height);
20.   heart.draw(Form1.Canvas,pHeart.x,pheart.y,true);
21. end;
22.  
23. procedure THeart.heartPosTimerTimer(Sender: TObject);
24. label 2;
25. begin
26.   if pHeart.y>=Form1.Height then
27.   begin
28.     FreeMe:=true;  GoTo 2;
29.   end;
30.   if (Player1.pPlayer.x<pHeart.x) and (pHeart.x<Player1.pPlayer.x+Player1.playerShip.Width) and (Player1.pPlayer.y<pHeart.y) and (pHeart.y<Player1.pPlayer.y+Player1.playerShip.Height) then
31.   begin
32.     Player1.Health:=Player1.Health+1;
33.     FreeMe:= true;
34.     Goto 2;
35.   end;
36.   pHeartFormer:=pHeart;
37.   pHeart.y:=pHeart.y+heartSpeed;
38.   heartPaint(pHeart,pHeartFormer);
39.   2:
40. end;
41.  

[Handoko]

Hello krull.

I really want to help you, but you can you please provide the "complete" source code? I can't compile and run it, so I can't test it. Using eye-inspection only is very hard. 

Copy all the necessary files (including the images) into a new folder, zip them and send it to this forum (using the attachment feature).

[Thaddy]

Yes. I think we need more code, because what you suggest (error when leaving the screen...) is not in the code you gave us.
So it must be something else.

[krull]

OK here the whole files!
puh 250 KB max. I shrink it extreme.

[krull]

Ok many Thanks, but I found it.
I have to disable the heartPosTimer. For me this seem to work, even if I don't know why.

[Eugene Loza]

Hi, krull!
I really like that you're making a computer game! It's really fun!
I'll make you some code suggestions, if you don't mind.

1. Never call Enemy1.destroy(); explicitly. Always use FreeAndNil(Enemy1) or Enemy1.Free.
2. "random( 8 )=8" will never happen, because Random( 8 ) provides random numbers from 0 to 7. 8 is not included.
3. Don't forget to FreeAndNil(heartPosTimer) in THeart.destroy.

Disabling heartPosTimer works because heartPosTimer calls back to THeart which may already be destroyed. Freeing heartPosTimer in THeart.destroy should solve this issue correctly. However, I'm not sure you're doing the right program "structure".
Maybe, you'd better make some global game-cycle which will draw all the sprites on-screen once per frame, not based on timers or threads, which are hard to debug.

[Handoko]

Glad to know you have solved the issue.

I tested your code but I can't make it work. Some advices hope it can be useful to you:

- When providing code created using Lazarus, you can ignore/delete the lib folder. The files inside that folder will be created automatically when user compile the code.

- To make your code cross platform (compilable on Linux) you should avoid using Windows unit. I didn't notice anything inside your code that 'strictly' requires Windows unit.

- Nothing wrong with using Goto statement but avoiding to use Goto will make code easier to debug especially for issues related with logic. Learn to avoid Goto is a good practice to improve your ability to solve problem with more organized way.

- You should avoid using hardcoding, instead use constant. Put your constant declarations near the top, you can easily to modify the code in the future. I saw you use hardcoding for file names, you will realize someday it is not good to do so.