PDA

View Full Version : TList.Items - what happens when you get out of range?



Darkhog
12-07-2013, 12:44 AM
What happens when your list has like 5 elements and you'll try to invoke e.g. 7th element (which doesn't exist)? Will you just get simple nil pointer or will program crash?

I need to know that because I have in Super Heli Land (link to GitHub repo if anyone interested: https://github.com/darkhog/SuperHeliLand) couple of TList-descendent classes (TSpriteList and TAnimatedSpriteList, sprites.pas) which due to implementation details of TGameObject can be accessed on accident with out of range values for List.Items.

I'm trying to come up with a better solution, but if all I'd get here is nil pointer it's isn't really problem as my code checks if we got nil on accident.

User137
12-07-2013, 04:22 AM
FPC version of TList at least raises an exception, and program propably stops to that completely.

function TFPList.Get(Index: Integer): Pointer;
begin
If (Index < 0) or (Index >= FCount) then
RaiseIndexError(Index);
Result:=FList^[Index];
end;

class procedure TFPList.Error(const Msg: string; Data: PtrInt);
begin
Raise EListError.CreateFmt(Msg,[Data]) at get_caller_addr(get_frame), get_caller_frame(get_frame);
end;

Andreaz
12-07-2013, 05:42 AM
User137 is correct, however, note that you can access elements up to the Capacity, but this might be even worse as you have no idea what they contain (They can contain a destroyed object or even an uninitialized pointer).

You should check the index before accessing the array instead :



procedure TGameObject.SetAnim(Value: Integer);
begin
_CurrentAnim:=Value; //setting actual animation value
if Animations.Items[_CurrentAnim]<>nil then Animations.Items[_CurrentAnim].Reset; //resetting animation if it wasn't nil so it'll play from start.
end;


becomes



procedure TGameObject.SetAnim(Value: Integer);
begin
_CurrentAnim:=Value; //setting actual animation value

if (_CurrentAnim >= 0) and (_CurrentAnim < Animations.Coint) then
begin
Animations.Items[_CurrentAnim].Reset; //resetting animation if it wasn't nil so it'll play from start.
end
end;


I removed the check for nil. that should not be necessary if all animations in the animation list is available.

laggyluk
12-07-2013, 06:15 AM
and what is the use of those lists?
i imagine that you are storing in some object a 'list index' as a reference and when list contents changes it fails. maybe consider storing a pointer instead?

Darkhog
13-07-2013, 12:16 AM
Instead of imagining things, look at my github repo: https://github.com/darkhog/SuperHeliLand, relevant code is under Sprites.pas.

//edit: @Andreaz, thanks. Though instead of checking if it's between, I'll just clamp _CurrentAnim in SetAnim so it'll always be between 0 and Count-1. Will also do this for sprites. Should be piece of cake.

//edit#2:

FPC version of TList at least raises an exception, and program propably stops to that completely.

function TFPList.Get(Index: Integer): Pointer;
begin
If (Index < 0) or (Index >= FCount) then
RaiseIndexError(Index);
Result:=FList^[Index];
end;

class procedure TFPList.Error(const Msg: string; Data: PtrInt);
begin
Raise EListError.CreateFmt(Msg,[Data]) at get_caller_addr(get_frame), get_caller_frame(get_frame);
end;

My list derives from TList, not TFPList (to maintain Delphi-compatibility - I want, if there would be such weirdo, person who would try to compile it under Delphi has as easy job porting as possible) so it's kinda unrelated here. But thanks for info.

SilverWarior
13-07-2013, 06:54 AM
My list derives from TList, not TFPList (to maintain Delphi-compatibility - I want, if there would be such weirdo, person who would try to compile it under Delphi has as easy job porting as possible) so it's kinda unrelated here. But thanks for info.

TFPList is class name for FreePascal implementation of TList. If I look at Delphi TList code TList.Get and TList.Error methods are exactly the same as the ones posted above.


As for Animations in general I would create another class for them and then connect your Sprite class to that so that your Sprite class only reads information from Animation class. By doing this you get an easy way of synchronizing animation between multiple sprites at once and for that you need only one call for each Animation type.

User137
13-07-2013, 11:47 PM
Yeah, it's actually interesting how it's defined

TList = class(TObject,IFPObserved)
private
FList: TFPList;
protected
function Get(Index: Integer): Pointer;
public
property Items[Index: Integer]: Pointer read Get write Put; default;
end; // + a bunch of stuff between those...

TList = class(TFPList) would be normal expected way, but this is different. I actually got valuable hint for nxPascal font class from this, on how to typecast object list from base-class. Through property... brilliant.