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 take ammo for most ranged spells #19

Closed
wants to merge 1 commit into from
Closed

Fix take ammo for most ranged spells #19

wants to merge 1 commit into from

Conversation

cala
Copy link
Contributor

@cala cala commented May 10, 2013

Fix spells like Arcane Shot not taking ammo while they should

This commit c653a66 is one of those pending backports from old mangos-zero concatenated in: https://github.com/cmangos/issues/wiki/classic_backporting-todo-zero-commits

stfx adviced to first apply to TBC, then backport to classic

I tested it and this is my report:

cala: tested take ammo and found issue on current revision: ammo are not taken for all spells (ex: ‘arcane shot’ does not take ammo but ‘serpent sting’ does). However, if no ammo in inventory no ranged spell can be casted, even those like ‘arcane shot’ that do not take ammo (though probably should take). I’ll try to test this commit.
stfx: was this behavior ever changed? it should be tested/fixed on tbc also if needed
cala (march 13-13): this fix does allow all spells to take ammo. It should be applied to TBC firsthand, then backported to classic.

Fix spells like Arcane Shot not taking ammo while they should

Signed-off-by: cala <calaftp@free.fr>
@Schmoozerd
Copy link
Member

How would this be on wotlk?
Because actually i do like the idea of refactoring the TakeAmmo code into a single function :)

@cala
Copy link
Contributor Author

cala commented May 11, 2013

I don't know if this is suitable or not for wotlk. I'll do some research regarding change in ammo between tbc and wotlk and I'll test with last wotlk core if this commit is relevant. Does that sound ok?

@Schmoozerd
Copy link
Member

that sounds great - thank you! ;)

@cala
Copy link
Contributor Author

cala commented May 12, 2013

Alright. Patch notes do not state big changes in ammunitions before cata expansion. I also tested wotlk and I think I noticed the same behavior than in TBC and classic. I said think because the character was shooting so fast a mix of autoshot and arcane shot, that it was hard to keep count of ammunition removed. Let's say I am 90% sure the behavior is the same in the three cores.
Thus, this commit should also applied for wotlk (at 90% 😉 )

Do you want that I close this pull request and open a new one for wotlk?

@Schmoozerd
Copy link
Member

hmm, i think i can simply take the patch from here as well :)
so a pull request should not be required
Poke @Stfx to remind me to not forget this here

rsa pushed a commit to mangosR2/mangos that referenced this pull request Jun 1, 2013
Fix spells like Arcane Shot not taking ammo while they should

Close cmangos/mangos-tbc#19

Signed-off-by: cala <calaftp@free.fr>
Signed-off-by: Schmoozerd <schmoozerd@cmangos.net>
Dramacydal pushed a commit to cmangos/mangos-cata that referenced this pull request Aug 19, 2013
Fix spells like Arcane Shot not taking ammo while they should

Close cmangos/mangos-tbc#19

Signed-off-by: cala <calaftp@free.fr>
Signed-off-by: Schmoozerd <schmoozerd@cmangos.net>

(based on commit [12510] - c8b2a94)

Signed-off-by: Dramacydal <PulLumBerMal@gmail.com>
Lillecarl referenced this pull request in Lillecarl/mangos-tbc Apr 11, 2016
Core/Unit: Don't split damage to targets that are immune.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants