Eluna icon indicating copy to clipboard operation
Eluna copied to clipboard

Add support for Loot Object

Open iThorgrim opened this issue 1 year ago • 3 comments

This PR introduces initial support for the Loot object within the Lua engine, specifically tailored for use with only TrinityCore 3.3.5 (for the moment). It adds methods to manipulate and interact with loot, including adding, removing, and checking items, as well as managing money and loot states.

Key Features:

  • Implemented Lua bindings for various Loot methods (e.g., AddItem, RemoveItem, IsLooted, GetMoney).
  • Integrated with existing Creature methods to handle loot retrieval (GetLoot, AllLootRemoved).
  • Added support for managing loot item counts, money, and looted states.

Note: This implementation is currently specific to TrinityCore and is not yet fully documented. Additional methods and documentation are planned for future updates.

Yes it's a Draft, if you'd like to test it and participate, i needs some help with the Mangos, Vmangos and Cmangos part. I think this kind of addition could be great for Eluna! (Like the SpellInfo method, by the way).

iThorgrim avatar Aug 11 '24 12:08 iThorgrim

Like mentioned in Discord, this will require either a weak pointer implementation for Loot, or we need to reintroduce partial dereferencing for objects that do not have a weak pointer available core side.

Maybe @Shauren and @Rochet2 has some thoughts on the matter.

Foereaper avatar Aug 11 '24 13:08 Foereaper

You would indeed have to change it to be owned by unique_trackable_ptr

My other concern with this PR is that it exposes Loot internals to scripts and lets users screw up easily

Creature::AllLootRemovedFromCorpse - this only signals the creature that all of its loot has been looted and it should update its corpse despawn time RemoveItem needs to not actually erase the item and instead flag the item as looted to make it disappear on client (and do a bunch of other bookkeeping stuff, basically do the same as Player::StoreLootItem but without storing the item anywhere) SetUnlootedCount and UpdateItemIndex should not be exposed at all SetItemLooted doesn't work for quest, ffa and conditional items (and taking into account my recommendation for RemoveItem these two functions should simply become one)

Shauren avatar Aug 11 '24 15:08 Shauren

You would indeed have to change it to be owned by unique_trackable_ptr

My other concern with this PR is that it exposes Loot internals to scripts and lets users screw up easily

Creature::AllLootRemovedFromCorpse - this only signals the creature that all of its loot has been looted and it should update its corpse despawn time RemoveItem needs to not actually erase the item and instead flag the item as looted to make it disappear on client (and do a bunch of other bookkeeping stuff, basically do the same as Player::StoreLootItem but without storing the item anywhere) SetUnlootedCount and UpdateItemIndex should not be exposed at all SetItemLooted doesn't work for quest, ffa and conditional items (and taking into account my recommendation for RemoveItem these two functions should simply become one)

What I've done for RemoveItem is that I've added a boolean 'force_remove' to give developper a choice.

Why not expose SetUnlootedCount and UpdateItemIndex? I use them for my AOE Loot script, for example, so maybe I didn't understand their purpose.

iThorgrim avatar Aug 11 '24 22:08 iThorgrim