Expired Loot Crash

[theFoof]

There is a crash where a player attempts to loot an item from an NPC that has despawned which results in an invalid pointer access of WorldCharacter::current_loot.

Easily reproduced, just go kill some random NPCs that respawn until you get one with an item drop. Keep the loot window open until after the NPC despawns, try to loot the item and crash.

Ratief mentioned that on live spawns would not disappear as long as the loot window was open. When nobody was looting a spawn that was supposed to have expired it would immediately despawn.

[zippyzee]

I thought I had safeguards in place to handle that but apparently not working. I'll take a look at it.

[zippyzee]

I'm pretty sure this is due to the memory cleanup of deleting the npc's loot inventory while the character still has a pointer to it.

I set this up to allow the loot window to stay up after the npc despawns, and looting to take place at any time afterward. In other words, when a player opens a loot window, he/she keeps a reference to that loot window for when they actually loot the npc.

I don't agree with not allowing a spawn to despawn when the loot window is open. That can easily allow player abuse by killing a needed quest mob and then 'sitting' on it, preventing others from doing the same. It also seems 'weird' to me from a gaming standpoint.

In a group setting there might be multiple group loot windows up all pointing to that loot inventory. Is there something clever we can do with shared pointers to prevent this from happening? Or perhaps make a copy of the loot inventory for the player, and delete the npc's copy when they despawn, leaving the player's copy as a regular variable that should clean up when the character is destroyed. Either way should work but you are certainly the memory-management expert.

EDIT: I took the simple route and now store a copy of the loot inventory on the player, rather than a pointer to the loot inventory, and all is well again. I'll push it in my commit today.

[theFoof]

Yup a shared_ptr would work, so that as long as a player has a copy of the ptr the loot object remains valid. And I'm fine with however you want to go about doing this, I knew it would take some design decisions so figured I'd post about it rather than just fix it. A copy might allow for multiple players to loot an item from the same NPC depending on how it is setup.

[zippyzee]

Vanguard won't allow a player to open a loot window if there is already one open (on that npc), so that solves that problem. Loot is probably my next focus so any info is good.

[OncaLupe]

[quote="theFoof"]Ratief mentioned that on live spawns would not disappear as long as the loot window was open. When nobody was looting a spawn that was supposed to have expired it would immediately despawn.[/quote]
That is correct, we did that all the time on raids. The Master Looter would open the mob's loot window, then we could take all the time we needed to determine who got what loot from it. If we kept it open long enough, the corpse would despawn as soon as the last item was given or the looter closed the window to let anything left rot. Though that usually only happened with trash mobs or in group setups as raid mobs had a very long corpse despawn time.

It would allow someone to sit on a corpse and not let it despawn, but the mob would still repop on it's own. The respawn timer was based on the time killed, not corpse despawn. So there was no issue of someone going AFK with the window open and preventing quest mob kills. I've seen on Live, mobs with a quick respawn time standing on their own corpse if loot was left behind (and I'm not talking about a quest triggered spawn).

Anyone in Unleashed would remember our first Tharridon kill. He still had a 15 min repop even though he was changed into a raid mob. We spent so long handling loot that he repoped on top of us. Fun times .

If you do go with the setup where the corpse despawns but loot window is still open (and most importantly active), then don't forget to make sure that loot rolling will still work. That closes the window and starts the roll.

[zippyzee]

Yes, I will have to keep in mind the raid practice of holding onto a corpse for looting purposes. This can be adjusted down the road a bit where necessary if it doesn't work as intended. Since group loot is not in yet, it is hard to make any final code for this.

[zippyzee]

Taking another look at this now. The loot bug about being able to loot items multiple times from a corpse arises from making a copy of the loot from the character's perspective. I will be adjusting this to move it more in line with this discussion.

[zippyzee]

I did a pretty major overhaul to this system (again). It is back to storing the loot only on the corpse, and I use various flags and variables to track who has an open loot window to the mob.

This now means that the unreal actor variable (is_looting) is being used for the character.

Now, when a character zones, if they are in the looting process the loot window will close. Previously, it would stay open and an attempt to loot would hang the loot system.

Now, when a character loots an interactive (as in a quest), they can't try to loot another interactive while the first window is open. This was never reported as a bug, but I found it when testing last night, and it would have been a problem.

Now, when a corpse has an open loot window, it will not despawn. There is a check in the despawning code to determine if the mob is being looted, and if the looting character is still in the chunk. If so, it will not despawn. If the character zones or exits the game, then the mob will despawn normally. The replacement mob will spawn on top of the corpse as described in previous posts.

I believe at this point we are replicating the loot process on live. Since loot has been a source of crashes previously, please be on the lookout for issues. I really tried to nail down as many contingencies as I could this time.

[Starcrusher]

Thanks ZZ!