Question about patch for issue 1236

29 views
Skip to first unread message

Stephen E. Baker

unread,
Apr 30, 2012, 3:35:10 PM4/30/12
to corsix...@googlegroups.com
Hello,

I wrote a patch that resolves issue 1236, however I wonder if it was
more verbose than necessary. The default distance for
findAllObjectsNear is 20, where the default distance for all the other
path finding functions is 2^30. Of course 20 was not enough to find all
the objects in large rooms, which the patch I attached to that issue
corrects by setting an appropriate distance for each room lookup.
However if the default distance had been sufficiently large, e.g. 2^30
like the others then it would have been unnecessary.

So my question is, was it defaulted to 20 on purpose (e.g. for
performance considerations; to be used in halls etc.) or was that a
typo, and my patch is unnecessary?

Either way, my patch is safe and has been tested, assuming room sizes
smaller than 32768x32768. (I think map sizes bigger than that would
cause other issues.)

Stephen E. Baker

Edvin Linge

unread,
May 2, 2012, 7:13:57 AM5/2/12
to corsix...@googlegroups.com
Hi!

I actually have no idea why it is smaller in that function. Looking at where
it is used the default should definitely be changed rather than special
distance arguments for each call. There is even one instance which already
calls it with 2^30. All calls to the function are made by rooms who want to
count or use all objects of one type, they will all fail if the room is too
big.

/Lego3

Stephen E. Baker

unread,
May 2, 2012, 8:39:24 AM5/2/12
to corsix...@googlegroups.com
Thanks Lego.

That's what I thought - the patch I wrote was just meant to be more
restricted in case it had been set to 20 for some future reason (and
because I hadn't bothered to follow up is isTilePartOfNearbyObject at
that time. I see it's always explicitly set to 10). I've attached a
new patch to the ticket based on this conversation.

Stephen E. Baker

P.S. When map sizes become variable we'll have to ensure they stay <=
2^15 x 2^15
Reply all
Reply to author
Forward
0 new messages