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

[4.3.4] Scripts/Paladin: Shield of the Righteous #9071

Merged
merged 1 commit into from Aug 24, 2014

Conversation

Projects
None yet
7 participants
Contributor

Xanvial commented Jan 27, 2013

Shield of the Righteous
Slam the target with your shield, causing Holy damage. Consumes all charges of Holy Power to determine damage dealt:

1 Holy Power: (X - 1) damage
2 Holy Power: (X * 3 - 3) damage
3 Holy Power: (X * 6 - 6) damage

http://www.wowpedia.org/index.php?title=Shield_of_the_Righteous&direction=prev&oldid=3036127
Last update in wowpedia before MoP

I delete entry in spell_bonus_data because it contains wotlk scaling (0% AP and SP)
Can't find how much it scaling now, so i hope dbc data is already correct

Member

DDuarte commented Jan 27, 2013

Live code review straight from the IRC!

[01:17] <%Nayd> Grey, got sum work for ya
[01:17] <%Nayd> review https://github.com/TrinityCore/TrinityCore/pull/9071/files#L1R792
[01:18] code looks fine, not sure about the math cuz i dont remember the damage mutplier per holy power for that spell
[01:18] <Vincent_Michael> "Ordered alphabetically using scriptname."
[01:18] btw Shocker and Shauren said that each time i used something like switch (caster->GetPower(POWER_HOLY_POWER)) in a Holy Power spell, i was giving cancer to someone in other coerner of the world
[01:19] so i stopped doing it
[01:19] lol
[01:19] <%Nayd> would
[01:19] <%Nayd> int hp = caster->GetPower(POWER_HOLY_POWER);
[01:19] <%Nayd> damage _= 0.5_hp_hp + 1.5_hp + 1;
[01:19] corner*
[01:19] <%Nayd> be better?
[01:19] mind bugs
[01:19] i'm not a math guy
[01:19] <%Nayd> :P
[01:20] if you say that its the same
[01:20] WHO I AM TO DISAGREE AMIRITE?
[01:20] <%Shauren> case 0: // 1 Holy Power
[01:20] <%Shauren> funny.
[01:20] <%Shauren> i thought you could have 0, 1, 2 or 3 holy power on retail
[01:20] <%Nayd> case 1 is 1 holy power?
[01:21] <%Shauren> but i think i can guess why its done this way - that 1 power required to cast was already taken
[01:21] <%Shauren> but no doc
[01:22] <%Shauren> document it, fuckers
[01:22] <%Nayd> he did the same in previous spell fix
[01:22] case 0 = 1 holy power
[01:22] case 1 = 2, case 2 = 3
[01:22] <%Shauren> that tells me nothing. probably doesnt to you either
[01:23] yup
[01:23] no idea why it works that way

Contributor

Xanvial commented Jan 27, 2013

so should i change to not use switch?

something like
damage = damage * 3 * caster->GetPower(POWER_HOLY_POWER);

nvm thats wrong, i'll use your formulas

Member

DDuarte commented Jan 27, 2013

In my opinion, the switch case is more clear than my formula.

Anyway, you really should explain why GetPower(HP) = 0 is 1 HP

Contributor

Xanvial commented Jan 27, 2013

well, for me, it's more clear when using switch.
Not a problem if you want to return using switch again :)

anyway, did this comment enough?

Contributor

Subv commented Jan 27, 2013

@DDuarte @Shauren Because the first holy power charge was taken by the spell cast in TakePower()

Member

DDuarte commented Jan 27, 2013

@Subv: that's not what the comment is saying

also, if the spell should consume all HP, why only one is taken?

Contributor

Xanvial commented Jan 27, 2013

Honestly i don't know why it works that way (0 for 1 HP, etc)

i'll try to find it out

Contributor

Xanvial commented Jan 27, 2013

Well, after looking around, maybe it's really because first HP charge taken by spell cast. As to why it consume all HP here, it's because how the handler work in here
https://github.com/TrinityCore/TrinityCore/blob/4.3.4/src/server/game/Spells/Spell.cpp#L4694

src/server/scripts/Spells/spell_paladin.cpp
+ // Because 1 Holy Power (HP) is consumed when casting spell,
+ // GetPower(POWER_HOLY_POWER) will return 0 when player has 1 HP,
+ // return 1 at 2 HP, and 2 at 3 HP
+ int hp = GetCaster()->GetPower(POWER_HOLY_POWER);
@Vincent-Michael

Vincent-Michael Jan 27, 2013

Member

int -> int32

@ghost ghost assigned Subv Jan 31, 2013

@ghost ghost assigned Warpten Mar 13, 2013

Member

jackpoz commented Aug 23, 2014

"Warpten: The commit itself seems fine"

jackpoz added a commit to jackpoz/TrinityCore that referenced this pull request Aug 24, 2014

@jackpoz jackpoz merged commit 9206890 into TrinityCore:4.3.4 Aug 24, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment