Skip to content

Commit

Permalink
Remove get_background_action with non-unique action names
Browse files Browse the repository at this point in the history
In the hunter module, get_background_action is often used with the presumable expectation to only create a single action per hunter (purely as an optimization). Since the background actions do not have pass on their constructor name to the action::name, that is not the case and the action get's constructed every time anyway. At the same time, the constructor name are just silently hidden instead to conform to the get_background_action.

Add an assert to the function, remove these non-intuitive usages.
  • Loading branch information
scamille committed Aug 4, 2024
1 parent 678ae44 commit a5ee04d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 18 deletions.
1 change: 1 addition & 0 deletions engine/class_modules/sc_demon_hunter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,7 @@ class demon_hunter_t : public parse_player_effects_t
}

auto action = new T( n, this, std::forward<Ts>( args )... );
assert( action->name_str == n && "Created background action does not match requested name" );
action->background = true;
background_actions.push_back( action );
return action;
Expand Down
37 changes: 19 additions & 18 deletions engine/class_modules/sc_hunter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ struct hunter_t final : public player_t
return dynamic_cast<T*>( *it );

auto action = new T( n, this, std::forward<Ts>( args )... );
assert( action->name_str == n && "Created background action does not match requested name" );
action -> background = true;
background_actions.push_back( action );
return action;
Expand Down Expand Up @@ -3809,7 +3810,7 @@ struct arcane_shot_t : public arcane_shot_base_t
{
struct arcane_shot_etf_t : public arcane_shot_base_t
{
arcane_shot_etf_t( util::string_view n, hunter_t* p ) : arcane_shot_base_t( p )
arcane_shot_etf_t( hunter_t* p ) : arcane_shot_base_t( p )
{
background = dual = true;
base_multiplier *= p->talents.eagletalons_true_focus->effectN( 3 ).percent();
Expand All @@ -3825,7 +3826,7 @@ struct arcane_shot_t : public arcane_shot_base_t

if ( p->talents.eagletalons_true_focus.ok() )
{
arcane_shot_etf = p->get_background_action<arcane_shot_etf_t>( "arcane_shot_etf" );
arcane_shot_etf = new arcane_shot_etf_t( p );
add_child( arcane_shot_etf );
}
}
Expand Down Expand Up @@ -3951,7 +3952,7 @@ struct serpent_sting_base_t: public hunter_ranged_attack_t
// Explosive Venom (Talent)
struct serpent_sting_explosive_venom_t final : public serpent_sting_base_t
{
serpent_sting_explosive_venom_t( util::string_view /*name*/, hunter_t* p ):
serpent_sting_explosive_venom_t( hunter_t* p ):
serpent_sting_base_t( p, "", p -> find_spell( 271788 ) )
{
dual = true;
Expand Down Expand Up @@ -3982,7 +3983,7 @@ struct explosive_shot_t : public hunter_ranged_attack_t
aoe = -1;
background = dual = true;

serpent_sting = p -> get_background_action<serpent_sting_explosive_venom_t>( "serpent_sting_explosive_venom" );
serpent_sting = new serpent_sting_explosive_venom_t( p );
}

void impact( action_state_t* s ) override
Expand Down Expand Up @@ -4087,7 +4088,7 @@ struct explosive_shot_background_t : public explosive_shot_t
{
size_t targets = 0;

explosive_shot_background_t( util::string_view, hunter_t* p ) : explosive_shot_t( p, "" )
explosive_shot_background_t( hunter_t* p ) : explosive_shot_t( p, "" )
{
dual = true;
base_costs[ RESOURCE_FOCUS ] = 0;
Expand Down Expand Up @@ -4442,7 +4443,7 @@ struct multishot_bm_t: public hunter_ranged_attack_t
aoe = -1;
reduced_aoe_targets = data().effectN( 1 ).base_value();

serpent_sting = p -> get_background_action<serpent_sting_explosive_venom_t>( "serpent_sting_explosive_venom" );
serpent_sting = new serpent_sting_explosive_venom_t( p );
}

void execute() override
Expand Down Expand Up @@ -4744,7 +4745,7 @@ struct chimaera_shot_t : public chimaera_shot_base_t
{
struct chimaera_shot_etf_t : public chimaera_shot_base_t
{
chimaera_shot_etf_t( util::string_view n, hunter_t* p ) : chimaera_shot_base_t( p )
chimaera_shot_etf_t( hunter_t* p ) : chimaera_shot_base_t( p )
{
background = dual = true;
base_multiplier *= p->talents.eagletalons_true_focus->effectN( 3 ).percent();
Expand All @@ -4760,7 +4761,7 @@ struct chimaera_shot_t : public chimaera_shot_base_t

if ( p->talents.eagletalons_true_focus.ok() )
{
chimaera_shot_etf = p->get_background_action<chimaera_shot_etf_t>( "chimaera_shot_etf" );
chimaera_shot_etf = new chimaera_shot_etf_t( p );
add_child( chimaera_shot_etf );
}
}
Expand Down Expand Up @@ -5372,7 +5373,7 @@ struct multishot_mm_base_t: public hunter_ranged_attack_t

if ( p -> talents.salvo.ok() )
{
explosive = p -> get_background_action<attacks::explosive_shot_background_t>( "explosive_shot_salvo" );
explosive = new attacks::explosive_shot_background_t( p );
explosive -> targets = as<size_t>( p -> talents.salvo -> effectN( 1 ).base_value() );
}
}
Expand Down Expand Up @@ -5439,7 +5440,7 @@ struct multishot_mm_t : public multishot_mm_base_t
{
struct multishot_mm_etf_t : public multishot_mm_base_t
{
multishot_mm_etf_t( util::string_view n, hunter_t* p ) : multishot_mm_base_t( p )
multishot_mm_etf_t( hunter_t* p ) : multishot_mm_base_t( p )
{
background = dual = true;
base_multiplier *= p->talents.eagletalons_true_focus->effectN( 3 ).percent();
Expand All @@ -5455,7 +5456,7 @@ struct multishot_mm_t : public multishot_mm_base_t

if ( p->talents.eagletalons_true_focus.ok() )
{
multishot_mm_etf = p->get_background_action<multishot_mm_etf_t>( "multishot_etf" );
multishot_mm_etf = new multishot_mm_etf_t( p );
add_child( multishot_mm_etf );
}
}
Expand Down Expand Up @@ -5501,7 +5502,7 @@ struct melee_focus_spender_t: hunter_melee_attack_t
{
struct serpent_sting_vv_t final : public serpent_sting_base_t
{
serpent_sting_vv_t( util::string_view /*name*/, hunter_t* p ):
serpent_sting_vv_t( hunter_t* p ):
serpent_sting_base_t( p, "", p -> find_spell( 259491 ) )
{
dual = true;
Expand Down Expand Up @@ -5534,7 +5535,7 @@ struct melee_focus_spender_t: hunter_melee_attack_t
hunter_melee_attack_t( n, p, s )
{
if ( p -> talents.vipers_venom.ok() )
vipers_venom_serpent_sting = p->get_background_action<serpent_sting_vv_t>( "serpent_sting_vv" );
vipers_venom_serpent_sting = new serpent_sting_vv_t( p );

wildfire_infusion_chance = p->talents.wildfire_infusion->effectN( 1 ).percent();
}
Expand Down Expand Up @@ -6269,7 +6270,7 @@ struct kill_command_t: public hunter_spell_t

struct explosive_shot_qs_t final : public attacks::explosive_shot_background_t
{
explosive_shot_qs_t( util::string_view /*name*/, hunter_t* p ) : explosive_shot_background_t( "", p )
explosive_shot_qs_t( hunter_t* p ) : explosive_shot_background_t( p )
{
base_dd_multiplier *= p->talents.sulfur_lined_pockets->effectN( 2 ).percent();
}
Expand Down Expand Up @@ -6322,7 +6323,7 @@ struct kill_command_t: public hunter_spell_t

if ( p->talents.sulfur_lined_pockets.ok() )
{
quick_shot.explosive = p->get_background_action<explosive_shot_qs_t>( "explosive_shot_quick_shot" );
quick_shot.explosive = new explosive_shot_qs_t( p );
add_child( quick_shot.explosive );
}
}
Expand Down Expand Up @@ -6800,7 +6801,7 @@ struct volley_t : public hunter_spell_t
background = dual = ground_aoe = true;

if ( p -> talents.salvo.ok() ) {
explosive = p -> get_background_action<attacks::explosive_shot_background_t>( "explosive_shot_salvo" );
explosive = new attacks::explosive_shot_background_t( p );
explosive -> targets = as<size_t>( p -> talents.salvo -> effectN( 1 ).base_value() );
}
}
Expand Down Expand Up @@ -6962,7 +6963,7 @@ struct wildfire_bomb_t: public hunter_spell_t
{
struct explosive_shot_grenade_juggler_t final : public attacks::explosive_shot_background_t
{
explosive_shot_grenade_juggler_t( util::string_view /*name*/, hunter_t* p ) : explosive_shot_background_t( "", p )
explosive_shot_grenade_juggler_t( hunter_t* p ) : explosive_shot_background_t( p )
{
base_dd_multiplier *= p->talents.grenade_juggler->effectN( 5 ).percent();
}
Expand Down Expand Up @@ -7081,7 +7082,7 @@ struct wildfire_bomb_t: public hunter_spell_t
if ( p->talents.grenade_juggler.ok() )
{
grenade_juggler.chance = p->talents.grenade_juggler->effectN( 2 ).percent();
grenade_juggler.explosive = p->get_background_action<explosive_shot_grenade_juggler_t>( "explosive_shot_grenade_juggler" );
grenade_juggler.explosive = new explosive_shot_grenade_juggler_t( p );
}
}

Expand Down
1 change: 1 addition & 0 deletions engine/class_modules/sc_rogue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,7 @@ class rogue_t : public player_t
}

auto action = new T( n, this, std::forward<Ts>( args )... );
assert( action->name_str == n && "Created background action does not match requested name" );
action->background = true;
background_actions.push_back( action );
return action;
Expand Down

0 comments on commit a5ee04d

Please sign in to comment.