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

fix (core): change order in HandleLootOpcode #159

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

acidmanifesto
Copy link
Contributor

I just can not understand why we have a check occuring after SendLoot. All checks should be before it in this case. Viewing gitblame, this was last touched back about 13 years ago and with no notes explaining the reasoning. Would love a further explaination in the reason why interrupt cast if statement is occurring after a send loot and not before.

Changes proposed

  • Change Order of GetPlayer()->SendLoot(guid, LOOT_CORPSE); to occur after checks and not inbetween checks within the HandleLootOpcode.

Issues addressed

  • Closes

SOURCE:

  • conditional if checks should be executed before the related action.

Tests performed

  • It does build with no issues in game from what i can see.

How to Test the Changes:

  1. I am able to craft multiple items with tayloring, and able to loot corpses just fine.
    cast spells or while crafting does get interrupted when i decide to loot something

Known issues and TODO list

  • [ ]
  • [ ]

  • Thank you for collaborating with the project.
  • For an efficient working methodology.
  • It is obvious to make long pull request, unless it is necessary.
  • A short pull request would be easy to test and approve.

@acidmanifesto acidmanifesto self-assigned this Aug 28, 2023
@seobryn seobryn merged commit 32193a8 into FirelandsProject:master Aug 29, 2023
3 checks passed
@acidmanifesto acidmanifesto deleted the op-order branch August 29, 2023 10:29
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.

2 participants