Core/Spells: Warrior fix Storm Bolt target selection #30827
Conversation
MoltenCrystal
changed the title
| Unit* caster = GetCaster(); | ||
| Unit* target = GetHitUnit(); | ||
|
|
||
| if (!caster || !target) |
There was a problem hiding this comment.
this is not necessary as caster and target are guaranteed to exist on OnEffectHitTarget
| return; | ||
|
|
||
| // Apply the stun effect to the primary target | ||
| caster->CastSpell(target, SPELL_WARRIOR_STORM_BOLT_STUN, true); |
There was a problem hiding this comment.
we also don't pass all cast flags anymore (true is full trigger flags) if possible, we pass required ones only
There was a problem hiding this comment.
Unsure on how trigger flags work as of now, where should I look or what should I use in this case?
| // Add the desired target back to the list | ||
| if (Unit* target = GetExplTargetUnit()) | ||
| { | ||
| targets.push_back(GetExplTargetUnit()); |
There was a problem hiding this comment.
there's no need for brackets for a single-liner
| { | ||
| Unit* caster = GetCaster(); | ||
|
|
||
| if (!caster) |
There was a problem hiding this comment.
caster is also guaranteed to exist at OnObjectAreaTargetSelect
| } | ||
|
|
||
| void Register() override | ||
| { | ||
| OnEffectHitTarget += SpellEffectFn(spell_warr_storm_bolt::HandleOnHit, EFFECT_1, SPELL_EFFECT_DUMMY); | ||
| OnObjectAreaTargetSelect += SpellObjectAreaTargetSelectFn(spell_warr_storm_bolt::FilterTargets, EFFECT_0, TARGET_UNIT_DEST_AREA_ENEMY); | ||
| OnEffectHitTarget += SpellEffectFn(spell_warr_storm_bolt::HandleDummy, EFFECT_1, SPELL_EFFECT_DUMMY); |
There was a problem hiding this comment.
hooked to the wrong effect as it currently stands: effect 1 uses TARGET_UNIT_TARGET_ENEMY which will only cast the stun spell on the explicit target regardless of clearing additional targets or not from effect 0 (clearing targets from one effect doesn't automatically apply to any other).
also, the wording of the talent is confusing since it doesn't state whether it should damage those additional targets or just stun them: if it's the former, hooking to effect 0 is fine; if it's the latter, then you gotta save those additional target guids from effect 0, then loop them in effect 1, etc., but it is likely not the case since the targetA is TARGET_UNIT_TARGET_ENEMY.
There was a problem hiding this comment.
Not quite there yet with hooks and handling, but shouldn't EFFECT_1 still be SPELL_EFFECT_DUMMY? Because if the caster has the talent [Storm Bolts] (https://www.wowhead.com/spell=436162/storm-bolts), it should hit, damage and stun two additional targets. But then I am not fully into what the "dummy" does right now either haha.
Any poiners to looping that? I was thinking about doing something with resize, but got told that it was a bad idea, and I am still learning how all of this works
Thank you so much for all comments!
…al/feature/warr_storm_bolt
…al/feature/warr_storm_bolt
|
i moved it to a separate script, makes it more maintainable, as its only related to that specific talent. |
Changes proposed:
Issues addressed:
Closes #30810
Tests performed:
Builds, and hits primary target like it now should unless talented in Storm Bolts
Known issues and TODO list: (add/remove lines as needed)