Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use_ring_action_added #339

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vigerus
Copy link

@Vigerus Vigerus commented Jan 4, 2024

Providing a code for rings usage by bots

list<Item*> rings;
list<Item*> result;

Player* bot = ai->GetBot();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a Player* bot defined in the values class, no need for this

Comment on lines +50 to +57
if (Item* ring1 = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_FINGER1))
rings.push_back(ring1);

if (Item* ring2 = bot->GetItemByPos(INVENTORY_SLOT_BAG_0, EQUIPMENT_SLOT_FINGER2))
rings.push_back(ring2);

if (rings.empty())
return result;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the list<Item*> rings is not needed, you should put the spell check and cooldown condition in a separate method and pass the ring there, if the method returns true then the ring is valid

{
public:
UseRingTrigger(PlayerbotAI* ai) : BoostTrigger(ai, "use ring", 3) {}
virtual bool IsActive() { return AI_VALUE(list<Item*>, "rings on use").size() > 0; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return !AI_VALUE(list<Item*>, "rings on use").empty();


bool UseRingAction::isPossible()
{
return AI_VALUE(list<Item*>, "rings on use").size() > 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return !AI_VALUE(list<Item*>, "rings on use").empty();

{
if (bot->CanUseItem(item) == EQUIP_ERR_OK && !item->IsInTrade())
{
return UseItemAuto(GetMaster(), item);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get the requester like this
Player* requester = event.getOwner() ? event.getOwner() : GetMaster();

Comment on lines +14 to +15
if (rings.empty())
return false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check for empty, the for already does it for you


for (Item * item : rings)
{
ItemPrototype const* proto = item->GetProto();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const ItemPrototype* proto = item->GetProto();

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In server files ItemPrototype const* proto is used

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmangos code doesn't follow any standards, but I'm trying to improve that atleast in our side

@@ -928,6 +928,13 @@ namespace ai
virtual bool IsActive() override;
};

class UseRingTrigger : public BoostTrigger
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to inherit from BoostTrigger if nothing gets reused from that class. Better inherit from Trigger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants