Planeshift contributor patch bug 2425, prevents crash viewing containers

With the contributor being me. weltall had a look and thinks it is OK. I would prefer if Kayden or someone knowing differences between EID/UID and FindObject/FindEntity had a look. 
THis probably prevents a bunch of crashes on LAanx (although it sounds similar other crashes near the same code occur). Would be nice if it could be applied on Laanx if this code is OK.

Adding link to bug here (will try to do that in better place next time :) ):
http://www.hydlaa.com/flyspray_upgrade/index.php?do=details&task_id=2425

[Khaki, 2008-11-01 19:58:24]

Looks fine but Kayden should check the cast.

http://codereview.appspot.com/7726/diff/1/2
File src/server/client.cpp (right):

http://codereview.appspot.com/7726/diff/1/2#newcode522
Line 522: gemObject *gemcont =
GEMSupervisor::GetSingleton().FindObject(EID(item->GetContainerID()));
Looks fine but I don't know if the cast is correct. Kayden would know about
this.

[Lanarel, 2008-11-01 21:43:31]

Added a comment where I copied my code change from.

http://codereview.appspot.com/7726/diff/1/2
File src/server/client.cpp (right):

http://codereview.appspot.com/7726/diff/1/2#newcode491
Line 491: gemObject *gemcont =
GEMSupervisor::GetSingleton().FindObject(EID(item->GetContainerID()));
Khaki: I 'copied' my code from here, where Kayden made a similar change. When
merging to stable/Laanx, this cast is not needed (as all are ints there).

[Lanarel, 2008-11-01 22:54:38]

Better fix to bug FS#2425 - Server crash when viewing container . Hopefully

[Lanarel, 2008-11-01 23:02:16]

Hopefully correct patch for Better fix to FS#2425 - Server crash when viewing
container . Hopefully

[Lanarel, 2008-11-01 23:06:46]

After vengeance convinced me that I did not know what I was doing and that the
original code made no sense anyway, I made a completely new patch for his
suggestion to just pass the gemContainer pointer to this function, as it is
always known outside.

[Khaki, 2008-11-02 00:07:03]

Looks good to me and committed in r2340. Please close.

[Kayden, 2008-11-02 08:32:30]

I was very surprised by this patch—I thought that CanTake was used when picking
up an item from the ground as well.  I see that isn't the case, however.  As
such, it probably makes sense for this to be a member of gemContainer instead of
Client.  Either that or the two operations should share some code after all.

[Lanarel, 2008-11-02 09:51:19]

Well, that explains a think in the code that was weird. If it could not find a
container ID, it would check if the item was guarded. Since this function was
only used for items in a container, the change vengeance suggested made sense,
and I even removed the possibility to use it for items outside a container. To
prevent confusion, moving it to gemContainer would make sense.

[Lanarel, 2008-11-02 11:49:18]

Nicer code for FS#2425 - Server crash when viewing container

[Lanarel, 2008-11-02 11:51:53]

Some clarification for my last set of patches:
- CanTake is cleaned up, combining several checks (or not doing them twice).
There is one left which I am not sure is needed (see comment in the function).
Possibly this check on item->IsNPCOwned etc can be removed.
- CanTake is moved from Client to gemContainer, as it only is used on containers

[Lanarel, 2008-11-02 13:42:31]

And another note to myself it seems :):
CanTake appears to be used not only for chests you drop, but more importantly
for items in furnaces and such. Which is why it should check the item is
guarded, and not the container. I 'accidentally' then do not check if the guard
is nearby the item, but do that for the container (preventing a crash). Tested
with furnace, all works fine.