Review for Arcane Mage's 11.1 Changes #9887
Conversation
|
should note that i hadnt removed av/concentration just to be safe -- don't think it gets called now, but its there. |
| @@ -3498,10 +3493,13 @@ struct arcane_barrage_t final : public dematerialize_spell_t | |||
| { | |||
| p()->trigger_clearcasting( 1.0, 0_ms ); | |||
| p()->trigger_arcane_charge( arcane_soul_charges ); | |||
| p()->buffs.arcane_soul_counter->trigger( p()->buffs.arcane_soul->remains() ); | |||
There was a problem hiding this comment.
The counter doesn't last that long, it can actually expire before Arcane Soul does.
| } | ||
|
|
||
| p()->trigger_spellfire_spheres(); | ||
| p()->trigger_mana_cascade(); | ||
| p()->consume_burden_of_power(); | ||
| consume_nether_precision( target, true ); |
There was a problem hiding this comment.
Calling consume_nether_precision with true as the 2nd argument doesn't make sense here. With AV gone, the second param should just be removed.
| } | ||
|
|
||
| p()->trigger_spellfire_spheres(); | ||
| p()->trigger_mana_cascade(); | ||
| p()->consume_burden_of_power(); |
There was a problem hiding this comment.
This triggers Glorious Incandescence, which is then immediately consumed. The order of these function calls needs to be adjusted.
| trigger_glorious_incandescence( s->target ); | ||
|
|
||
| if ( ( s->n_targets > 2 ) && result_is_hit( s->result ) && p()->talents.arcane_rebound.ok() ) |
There was a problem hiding this comment.
This would trigger the Rebound on every target rather than just the primary one.
There was a problem hiding this comment.
Also, hardcoding the target threshold is bad practice.
| @@ -3965,6 +3970,15 @@ struct arcane_missiles_t final : public custom_state_spell_t<arcane_mage_spell_t | |||
| { | |||
| p()->buffs.clearcasting_channel->trigger(); | |||
| p()->trigger_time_manipulation(); | |||
| p()->buffs.intuition->trigger(); | |||
|
|
|||
| if ( rng().roll( p()->talents.leydrinker->effectN( 1 ).percent() ) ) | |||
There was a problem hiding this comment.
What about Arcane Explosion? This is the wrong place to trigger Leydrinker.
| struct arcane_rebound_t final : public arcane_mage_spell_t | ||
| { | ||
| arcane_rebound_t( std::string_view n, mage_t* p ) : | ||
| arcane_mage_spell_t( n, p, p->find_spell( 210817 ) ) |
There was a problem hiding this comment.
This is the old Legion spell; in-game it uses 1223801.
| @@ -6069,6 +6084,16 @@ struct meteorite_impact_t final : public mage_spell_t | |||
| } | |||
| }; | |||
|
|
|||
| struct arcane_rebound_t final : public arcane_mage_spell_t | |||
There was a problem hiding this comment.
This needs to derive from spell_t since it doesn't benefit from any of the mage effects.
| @@ -8543,6 +8574,8 @@ void mage_t::create_buffs() | |||
| // Sunfury | |||
| buffs.arcane_soul = make_buff( this, "arcane_soul", find_spell( 451038 ) ) | |||
| ->set_chance( specialization() == MAGE_ARCANE && talents.memory_of_alar.ok() ); | |||
| buffs.arcane_soul_counter = make_buff( this, "arcane_soul_counter", find_spell( 1223522 ) ) | |||
| ->set_chance( specialization() == MAGE_ARCANE && talents.memory_of_alar.ok() ); | |||
There was a problem hiding this comment.
You're not setting the default value here. The buff does nothing.
| ->set_default_value_from_effect( 1 ) | ||
| ->set_chance( sets->set( MAGE_ARCANE, TWW1, B4 )->effectN( 1 ).percent() ); | ||
| ->set_chance( find_spell( 1223798 )->effectN( 1 ).percent() ); |
There was a problem hiding this comment.
This is missing the talent check. It just triggers regardless of whether you have the talent or not.
| @@ -3965,6 +3970,15 @@ struct arcane_missiles_t final : public custom_state_spell_t<arcane_mage_spell_t | |||
| { | |||
| p()->buffs.clearcasting_channel->trigger(); | |||
| p()->trigger_time_manipulation(); | |||
| p()->buffs.intuition->trigger(); | |||
There was a problem hiding this comment.
Even though AM cannot be cast without CC, you shouldn't assume this will always be the case. Intuition wouldn't work properly then.
No description provided.