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

Already on GitHub? Sign in to your account

Core/Spells: Fixed Soulwell charges removal on use. #7839

Merged
merged 1 commit into from Oct 5, 2012

Conversation

Projects
None yet
3 participants
Member

Warpten commented Sep 22, 2012

What's the bug

  • Warlock's soulwell should not lose charges if you cannot store a stone in your inventory, either because you don't have free space or because you already have a stone.
  • Soulwell shouldn't lose two charges per use.

What does the fix fix

  • Fixes charges loss

What does the fix bug out

  • No matter what, you will always get min. one, max. two SpellCastResult error strings. The reason for this is that the Soulwell sends CMSG___REPORT_USE and CMSG___USE. The two packet handlers related to these call GameObjectAI::GossipHello. So we have a race between the GossipHello hook called by REPORT_USE and the one called by USE.

Case 1:
a) Player has no stone.
b) Player clicks on the well.
c) First call of the hook will add the stone, second one will send an error string (SpellCastResult)
d) The well loses 1 charge.
You now have one stone, and one error string.

Case 2:
a) Player has a stone.
b) Player clicks on the well.
c) First call of the hook will print the error string, second one will do the same.
d) Charges are not affected.
You now have one stone, and two error strings.

Why using the boolean returned by GossipHello ?

It was designed for that, and it'll prevent, in the case of the soulwell, GameObject::Use from removing a charge itself later on (See case GAMEOBJECT_TYPE_SPELLCASTER).

What is the correct fix ?

We need to somewhat have access to the SpellCastResult in Unit::CastSpell in order to determine if charges are going to be effectively removed. I can address that in another patch if i get a "GO" message from any dev.

Closes #7397.

Star-lion added a commit that referenced this pull request Oct 5, 2012

Merge pull request #7839 from Warpten/soulwell
Core/Spells: Fixed Soulwell charges removal on use.

@Star-lion Star-lion merged commit 42c047b into TrinityCore:master Oct 5, 2012

Contributor

Jildor commented Nov 16, 2012

The fix it's not working properly #8325

raczman pushed a commit to raczman/TrinityCore that referenced this pull request Apr 20, 2014

Merge pull request #7839 from Warpten/soulwell
Core/Spells: Fixed Soulwell charges removal on use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment