Release loot at end of HandleAutostoreLootItemOpcode

[BlessedCammi]

🍰 Pullrequest

Resolve dup involving opening chests in inventory or chest objects. Player can open a chest at the same time as forcing a logout or teleport (making player leave world) to restore contents of chest item. This pull request forces the server to handle the looting of item.

I'm going through my repo changes and I'm pretty sure this was the only thing we needed to resolves the dupe from our end.

Tested in main repo as well to make sure it wasn't something from just our core.

To replicate - Open a chest item from inventory with autoloot enabled, whilst disconnecting or going through a portal. Timing has to be right.

Issues

• None

[killerwife]

Q:

Can code in WorldSession::HandleLootMasterGiveOpcode which also has Loot::SendItem also be used to the same end?

[BlessedCammi]

Q:

Can code in WorldSession::HandleLootMasterGiveOpcode which also has Loot::SendItem also be used to the same end?

Probably, good catch. Someone could probably give themselves the item in a chestobject via masterloot and force a disconnect at the same time. I'd have to test though to confirm. It would have to be the last loot item in the loot table to force it's contents to repopulate.

[BlessedCammi]

Can't replicate with masterloot but it may be a timing thing which isn't easy to replicate for this scenario.

[BlessedCammi]

oof the indenting or spacing is cooked and I don't know how to sort it out on github. Also another possible dup fixed.

[killerwife]

That one i dont think is valid. You still launch a roll during master loot if master looter isnt eligible for it.

[BlessedCammi]

That one i dont think is valid. You still launch a roll during master loot if master looter isnt eligible for it.

What scenario would be the case where masterloot isn't eligible? I guess if the masterlooter isn't present in the same map or online. Could a if no masterLooter variable check work instead? Not sure if that fits all scenarios though.

[killerwife]

Too far on kill, not in map. And i dont remember how below the threshold works for master looter.

[BlessedCammi]

Too far on kill

Ya this is one aspect I was thinking a no masterLooter check wouldn't catch too.

[BlessedCammi]

Actually nvm, the

for (auto playerGuid : m_ownerSet)
{
	Player* player = sObjectAccessor.FindPlayer(playerGuid); 

handles this, maybe just a

if(masterLooter)
  break;

would work before the group loot roll out.

[BlessedCammi]

oof I dont know how to work around the formatting using the browser as an editor with copy paste from visual. Sorry I've mega cooked the formatting on this pull request.

[BlessedCammi]

Actually last commit doesn't work because it checks all players within the loop. 😖
Needs a more refined approach. Imma leave it for now, I need to make food. The nuclear option works for now 🤣

[killerwife]

Let me worry about the rest. It will take me some time to merge this, but ill do a bit of a review of the existing loot code at the same time, since adjusting loot code is a bit of a landmine.

[evil-at-wow]

And i dont remember how below the threshold works for master looter.

IIRC, that loot is assigned to someone just like in the group loot case. So for example if you're master looter for rare+ (blue+) items, and we kill say Onyxia and her loot would be assigned to 'me', then I can just take the backpack and sack of gems (common/green items) while you'll have to hand out the rest of the loot as master looter. At least that's how I tested it to be at one point in time (I think when 5.x was current, so still take it with a grain of salt for TBC/Classic).

[killerwife]

So below threshold round robin, above threshold master looter, and above threshold when ML wasnt in range, group loot