Skip to content

Clean up unused variables#2268

Open
Celandriel wants to merge 6 commits intomod-playerbots:test-stagingfrom
Celandriel:moreUnusedChanges
Open

Clean up unused variables#2268
Celandriel wants to merge 6 commits intomod-playerbots:test-stagingfrom
Celandriel:moreUnusedChanges

Conversation

@Celandriel
Copy link
Copy Markdown
Collaborator

@Celandriel Celandriel commented Mar 30, 2026

Pull Request Description

Clean up a bunch of additional unused variable warnings.

Feature Evaluation

  • Describe the minimum logic required to achieve the intended behavior.
  • Describe the processing cost when this logic executes across many bots.

How to Test the Changes

Impact Assessment

  • Does this change increase per-bot/per-tick processing or risk scaling poorly with thousands of bots?

      • No, not at all
      • Minimal impact (explain below)
      • Moderate impact (explain below)
  • Does this change modify default bot behavior?

      • No
      • Yes (explain why)
  • Does this change add new decision branches or increase maintenance complexity?

      • No
      • Yes (explain below)

Messages to Translate

  • Does this change add bot messages to translate?
      • No
      • Yes (list messages in the table)
Message key Default message

AI Assistance

  • Was AI assistance used while working on this change?
      • No
      • Yes (explain below)

Claude reviewed the warnings log from a build and suggested a series of changes. I focused just on these warnings for now. Every line was reviewed. Some sections need to be reviewed by author for intent.

Final Checklist

    • Stability is not compromised.
    • Performance impact is understood, tested, and acceptable.
    • Added logic complexity is justified and explained.
    • Documentation updated if needed (Conf comments, WiKi commands).

Notes for Reviewers

@Celandriel
Copy link
Copy Markdown
Collaborator Author

@brighton-chi @kadeshar Can you review if any signatures of code youve written (strategies) should be modified? I understand not modifying old code, but for strategies youve written, if its not needed, then consider suggesting a change in signature.

@brighton-chi
Copy link
Copy Markdown
Contributor

Yeah, seems I missed some that weren't needed anymore after I changed stuff around. Definitely can just modify the signatures instead of commenting out.

Comment thread src/Ai/Raid/SerpentshrineCavern/Action/RaidSSCActions.cpp Outdated
@Celandriel Celandriel marked this pull request as ready for review April 2, 2026 19:43
@brighton-chi
Copy link
Copy Markdown
Contributor

brighton-chi commented Apr 5, 2026

Also both variables should be deleted from bool HasFireBombNearby(PlayerbotAI* botAI, Player* bot)

@kadeshar
Copy link
Copy Markdown
Collaborator

kadeshar commented Apr 6, 2026

Conflicts + compilation checks to resolve

Copy link
Copy Markdown
Collaborator Author

@Celandriel Celandriel left a comment

Choose a reason for hiding this comment

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

Merged.

@kadeshar
Copy link
Copy Markdown
Collaborator

kadeshar commented Apr 9, 2026

Compilation error

}

bool MovementAction::IsMovingAllowed(uint32 mapId, float x, float y, float z)
bool MovementAction::IsMovingAllowed(uint32 /*mapId*/, float /*x*/, float /*y*/, float /*z*/)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This parameters should be removed not commented

}

bool MovementAction::IsDuplicateMove(uint32 mapId, float x, float y, float z)
bool MovementAction::IsDuplicateMove(uint32 /*mapId*/, float x, float y, float z)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This parameter should be removed not commented

}

bool MovementAction::ChaseTo(WorldObject* obj, float distance, float angle)
bool MovementAction::ChaseTo(WorldObject* obj, float distance, float /*angle*/)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This parameter should be removed not commented

}

void ReleaseSpiritAction::LogRelease(const std::string& releaseMsg, bool isAutoRelease) const
void ReleaseSpiritAction::LogRelease(const std::string& releaseMsg, bool /*isAutoRelease*/) const
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This parameter should be removed not commented

}

void ChatReplyAction::ChatReplyDo(Player* bot, uint32& type, uint32& guid1, uint32& guid2, std::string& msg, std::string& chanName, std::string& name)
void ChatReplyAction::ChatReplyDo(Player* bot, uint32& type, uint32& guid1, uint32& /*guid2*/, std::string& msg, std::string& chanName, std::string& name)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This parameter should be removed not commented

Check rest for same.

@Celandriel
Copy link
Copy Markdown
Collaborator Author

It was said last time we did this thag we should try to avoid editing the signatures.
I was comfortable doing it for code that we have added recently. I was less sure about the cases you identified.

@kadeshar
Copy link
Copy Markdown
Collaborator

kadeshar commented Apr 17, 2026

Like before im against such fixes #2106. That only hide real problem and not fix anything. Claming that are maybe required to production solution also doesnt make sense. Then comment not only name but also type. When will be required on production solution someone only uncomment them. But im almost certain that will never happen xD

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