PDA

View Full Version : TLists (resolved)



dazappa
07-11-2009, 10:52 PM
First, Stoney gave me this very helpful link: http://www.delphibasics.co.uk/RTL.asp?Name=TList

I've got stuff being added and removed from the list just fine, but it seems that it's only removing the pointers, as the memory usage of my program rises indefinitely.

This is the code I use to remove the clouds (What's in the TList)
(Where clouds[] is the TList, and TCloud is the type of object inserted into the list)

TCloud(clouds[i]).Free;
clouds.delete(i);


(Random rambling below):
Well I've been using Andorra 2D as the engine in my PGD entry, but the built in collision system won't compile in... :| Kind of necessary to make a game...

User137
08-11-2009, 12:48 AM
What does the loop look like where objects are being deleted? If you loop them from start to end it will not remove all of them. Otherwise i can't think of a memory hole. Take care of freeing the list itself, and see if there is duplication happening in creating object, or making instance and losing grip of it.

dazappa
08-11-2009, 01:14 AM
Creation

procedure CLOUDCTRL.onTimer(Sender: TObject);
var
cloud: TCloud;
begin
Cloud:=TCloud.Create;
cloud.img := random(2);
cloud.x := 800;
cloud.y := -30+random(105);
cloud.speed := 1+random(2);
clouds.Add(cloud);
cloudtimer.Interval:=2000+random(4000);
end;


Destruction:

i:=0;
while(i <= clouds.count-1) do
begin
tmpcloud := TCloud.create;
tmpcloud := TCloud(clouds[i]);
tmpcloud.x := tmpcloud.x-tmpcloud.speed;

if(tmpcloud.img=0) then
begin
locallist.Find('cloud1').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+151,tmpcloud.y+77),0,150);
end else
begin
locallist.Find('cloud2').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+183,tmpcloud.y+79),0,150);
end;
if(tmpcloud.x <= -185) then { Actual Deletion Here }
begin
TCloud(clouds[i]).Free;
clouds.delete(i);
i:=i-1;
end;
i:=i+1;
end;

I've already tested and confirmed that they're being deleted from the list... the problem is the "real" objects, as the TList seems to only be a list of pointers.

JSoftware
08-11-2009, 08:14 AM
Destruction:

tmpcloud := TCloud.create;
tmpcloud := TCloud(clouds[i]);



Here's your problem. You allocate a new cloud and then you get a cloud instance from the list. The cloud you allocated is then lost, and the memory has been leaked

Just do tmpcloud := TCloud(clouds[i]); without the TCloud.Create before it. tmpcloud is just a pointer, you don't have to allocate storage for an object or something like that

dazappa
09-11-2009, 08:48 PM
Destruction:

tmpcloud := TCloud.create;
tmpcloud := TCloud(clouds[i]);



Here's your problem. You allocate a new cloud and then you get a cloud instance from the list. The cloud you allocated is then lost, and the memory has been leaked

Just do tmpcloud := TCloud(clouds[i]); without the TCloud.Create before it. tmpcloud is just a pointer, you don't have to allocate storage for an object or something like that

I don't think tmpcloud itself is a pointer, as it's defined "var tmpcloud: TCloud". However, removing the .create line resulted in the app seg faulting. I haven't really used pointers, so the only thing I could think of to make tmpcloud a pointer would be to define it as "var tmpcloud: ^TCloud", but that's apparently not enough...

JSoftware
09-11-2009, 11:06 PM
Is tcloud a class or an object?

paul_nicholls
10-11-2009, 12:07 AM
Destruction:

tmpcloud := TCloud.create;
tmpcloud := TCloud(clouds[i]);



Here's your problem. You allocate a new cloud and then you get a cloud instance from the list. The cloud you allocated is then lost, and the memory has been leaked

Just do tmpcloud := TCloud(clouds[i]); without the TCloud.Create before it. tmpcloud is just a pointer, you don't have to allocate storage for an object or something like that

I don't think tmpcloud itself is a pointer, as it's defined "var tmpcloud: TCloud". However, removing the .create line resulted in the app seg faulting. I haven't really used pointers, so the only thing I could think of to make tmpcloud a pointer would be to define it as "var tmpcloud: ^TCloud", but that's apparently not enough...


If TCloud is defined like so:

TCloud = Class(TObject) // or just Class
...
End;


then, TCloud is a pointer automatically, as all object instances are pointers behind the scenes, and they are de-referenced automatically so you never need the ^ de-reference when dealing with classes.

With regards to the code below:


tmpcloud := TCloud.create;
tmpcloud := TCloud(clouds[i]);


If removing the first line causes it to crash, then I guess the TCloud instance you are retrieving from the List at position i is not valid so when you use it later on, BOOM!!

It was either never created in the first place, or has been freed somehow prior to you retrieving and attempting to use it at this point.

I hope some of this helps? :)

cheers,
Paul

User137
10-11-2009, 10:47 PM
In finding the cause to segment fault you may need to change the while loop into

while (i <= clouds.count-1) and (clouds.count>0) do
...

Otherwise tmpcloud := TCloud(clouds[i]); will result in error when there is no clouds.

and yes, you need to remove tmpcloud := TCloud.create;
Note that all calls to tmpcloud after assign will actually change clouds in the TList, you only give it easier "name" that is pointer.

dazappa
11-11-2009, 01:00 AM
In finding the cause to segment fault you may need to change the while loop into

while (i <= clouds.count-1) and (clouds.count>0) do
...

Otherwise tmpcloud := TCloud(clouds[i]); will result in error when there is no clouds.

and yes, you need to remove tmpcloud := TCloud.create;
Note that all calls to tmpcloud after assign will actually change clouds in the TList, you only give it easier "name" that is pointer.

Ah, that makes sense of paul's last part of his post. And, as expected, the seg faults are gone ;). I would have expected a different error, (array index out of bounds), but this isn't exactly an array.



If TCloud is defined like so:

TCloud = Class(TObject) // or just Class
...
End;


then, TCloud is a pointer automatically, as all object instances are pointers behind the scenes, and they are de-referenced automatically so you never need the ^ de-reference when dealing with classes.

Thanks :) That's a useful tidbit to know.

Also: Yes, now that it's no longer creating a new cloud the constant memory rise is no more ;)

User137
11-11-2009, 06:52 AM
On second thought the previous while loop must've been ok too...

But i'd do with for loop, optimizing list deletion faster same time:

for i:=clouds.count-1 downto 0 do
begin
tmpcloud := TCloud(clouds[i]);
tmpcloud.x := tmpcloud.x-tmpcloud.speed;

if(tmpcloud.img=0) then
begin
locallist.Find('cloud1').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+151,tmpcloud.y+77),0,150);
end else
begin
locallist.Find('cloud2').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+183,tmpcloud.y+79),0,150);
end;
if(tmpcloud.x <= -185) then { Actual Deletion Here }
begin
TCloud(clouds[i]).Free;
clouds[i]:=clouds[clouds.Count-1];
clouds.delete(clouds.Count-1); // deleting last index prevents
// moving of all last clouds in the array forward
// but it changes drawing order which isn't usually a problem
end;
end;

dazappa
11-11-2009, 12:10 PM
On second thought the previous while loop must've been ok too...

But i'd do with for loop, optimizing list deletion faster same time:

for i:=clouds.count-1 downto 0 do
begin
tmpcloud := TCloud(clouds[i]);
tmpcloud.x := tmpcloud.x-tmpcloud.speed;

if(tmpcloud.img=0) then
begin
locallist.Find('cloud1').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+151,tmpcloud.y+77),0,150);
end else
begin
locallist.Find('cloud2').DrawAlpha(AdDraw,AdRect(t mpcloud.x,
tmpcloud.y,tmpcloud.x+183,tmpcloud.y+79),0,150);
end;
if(tmpcloud.x <= -185) then { Actual Deletion Here }
begin
TCloud(clouds[i]).Free;
clouds[i]:=clouds[clouds.Count-1];
clouds.delete(clouds.Count-1); // deleting last index prevents
// moving of all last clouds in the array forward
// but it changes drawing order which isn't usually a problem
end;
end;

I thought I read somewhere that for loops were less efficient than while loops, which is why I've been using them. I'll try the optimized deletion later today.