PlaneShift

Another EID vs. UID bug - luckily those are easy to fix :)

NOTE: Do not commit this patch before ALL related problems are solved!

Index: src/server/bulkobjects/psitem.cpp
===================================================================
--- src/server/bulkobjects/psitem.cpp	(revision 2638)
+++ src/server/bulkobjects/psitem.cpp	(working copy)
@@ -74,10 +74,10 @@
 class psItemRemovalEvent : public psGameEvent
 {
 protected:
-    EID item_to_remove;
+    uint32 item_to_remove;
 
 public:
-    psItemRemovalEvent(int delayticks, EID gemID)    
+    psItemRemovalEvent(int delayticks, uint32 gemID)    
         : psGameEvent(0,delayticks*1000,"psItemRemovalEvent")
     {
         item_to_remove = gemID;
@@ -86,9 +86,9 @@
 
     virtual void Trigger()
     {
-        Debug2(LOG_USER, 0, "Removing object %s now.\n", ShowID(item_to_remove));
+        Debug2(LOG_USER, 0, "Removing object %d now.\n", item_to_remove);
         // cannot store pointer because object may have already been removed and ptr not valid
-        gemObject *obj = GEMSupervisor::GetSingleton().FindObject(item_to_remove);
+        gemObject *obj = GEMSupervisor::GetSingleton().FindItemEntity(item_to_remove);
         if (obj)
         {
             psItem *item = obj->GetItem();

Edit 07.12.2008: Updated patch to work with current trunk.

Peeg, why should this patch not be committed? It does not seem to be related to the other parts. If this report is to be used as a collection of transient bugs, maybe the patch should have its own report? I know Kayden just fixed two similar bugs (for items in crates).

it is more if this is commited not to merge to stable till the others are included so that we dont have players complaining of issues with missing items or odd things happening

What Caarrie said… My patch solves exactly the main problem: "TRANSIENT flag doesn't work", but there are a lot of related problems.
If this patch makes it into the next release before at least  FS#2250  is fixed we'll have a lot of players go berserk because all their items will start to disappear from their (previously safe) guildhouses.

OK I understand. It is just that that same change in the 'opening crate crashes server' thing, caused a crash, because FindItemEntity was used instead of FindObject. Even if the patch seems to do the opposite here, since the two do not seem to be interchangeable, I assume not applying this patch will mean the server crashes on this at this moment. So I wondered why not prevent a crash from happening.

Combining that with what you two say, therefor translates to: we will let the server crash when it tries to remove a transient object to prevent people missing objects due to other things not being implemented yet. Until someone tells me the server does not crash on this at this moment, I will increase severity from low to flash :)
This may mean it should quickly be solved by disabling transient objects another way (and lower priority again), if fixing all other things takes too long.

Lanarel we never said there was a crash related to this, just the flag did not work correctly, please change the severity/priority back ;)

I know you did not say it, but judging from what I know about the difference between findObject and findItemEntity, there may very well be a crash caused by this that no-one noticed. SOmeone convince me that current code does not cause a crash and severity can go down. THen someone has to convince us that Peegs patch will not introduce a crash before it gets implemented. As far as I know these two commands are not interchangeable, and one of the two will cause crashes :)

No possible crash here. Neither with the current code, nor with my patch. :)
GEMSupervisor.FindObject() and GEMSupervisor.FindItemEntity() are both returning NULL if no matching object is found and since the variable "obj" is checked for NULL cleanly there's no crash.

Peeg, please have a look at  bug 2272 . The problem there was that at least one of those functions did not return NULL but an invalid address, causing a crash. THe input argument of the functions (as you know) does not have the same meaning, but for some reason you can pass them to both functions (as they are really just integers). And even when it not crashes, it will sometimes not return null but another object. This may mean currently if you place an apple for an hour, after an hour a furnace vanishes :). Or I am completely wrong, in which case I just need better convincing than "you are wrong":)

Peeg convinced me that it will not crash, but I think I am still right in assuming that means that random other items will be disappearing. If the other issues related to this transient stuff can not be fixed soon, I suggest the feature to be disabled completely on laanx asap (hence leaving priority), to prevent furniture disappearing from guildhouses and other bugs.

Decreasing severity, as this will not effect the game as long as it is not enabled, and the functionality itself is not critical.

Again, passing this to Ares. He can decide how to handle it in his hierarchy of things to do for gms.

Please consult with Peeg, who looked into these transient things before.

Note when testing, you must now move far away from the dropped item as it wont disappear while your guarding it (PS#2253).

Is transient working at all right now? Because for testing purposes I have some transient items lying here in my NPCroom and they just won't disappear.

Peeg's patch to enable transient item still hasn't been committed, if you want to test transients apply his patch (1st comment). We're not confident that all issues related to transients have been resolved so this may not be enabled for a while.