From 59c203618f7b1e74922f5d7213409814c75dbeee Mon Sep 17 00:00:00 2001 From: Kate Martin <51387586+renanthera@users.noreply.github.com> Date: Thu, 19 Jun 2025 23:35:23 -0600 Subject: [PATCH 1/4] [sim] Allow to skip `init_finished()` and prevent expr validation to save APLs with invalid expressions. --- engine/sc_main.cpp | 2 +- engine/sim/sim.cpp | 6 +++++- engine/sim/sim.hpp | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/engine/sc_main.cpp b/engine/sc_main.cpp index cbc1d0c22bd..2e20e029f17 100644 --- a/engine/sc_main.cpp +++ b/engine/sc_main.cpp @@ -147,7 +147,7 @@ sim_t* sim_signal_handler_t::global_sim = nullptr; bool need_to_save_profiles( sim_t* sim ) { - if ( sim->save_profiles ) + if ( sim->save_profiles || sim->save_profile_pre_init ) { return true; } diff --git a/engine/sim/sim.cpp b/engine/sim/sim.cpp index f5df6d92e31..593d3544c61 100644 --- a/engine/sim/sim.cpp +++ b/engine/sim/sim.cpp @@ -1397,6 +1397,7 @@ sim_t::sim_t() initialized( false ), fixed_time( true ), save_profiles( false ), + save_profile_pre_init( false ), save_profile_with_actions( true ), save_full_profile( true ), default_actions( false ), @@ -2900,7 +2901,9 @@ void sim_t::init() // We are committed to simulating something. Tell actors that the sim init is now complete if they // need to do something. - if ( ! canceled ) + // Do not call init_finished if saving profiles without validation, or program + // possibly exits due to invalid expr exception. + if ( ! canceled && !save_profile_pre_init ) { bool verify_use_items_state = true; @@ -3718,6 +3721,7 @@ void sim_t::create_options() add_option( opt_timespan( "ignite_sampling_delta", ignite_sampling_delta ) ); // Output add_option( opt_bool( "save_profiles", save_profiles ) ); + add_option( opt_bool( "save_profile_pre_init", save_profile_pre_init ) ); add_option( opt_bool( "save_profile_with_actions", save_profile_with_actions ) ); add_option( opt_bool( "save_full_profile", save_full_profile ) ); add_option( opt_bool( "default_actions", default_actions ) ); diff --git a/engine/sim/sim.hpp b/engine/sim/sim.hpp index cec5bfb2a05..d295168b081 100644 --- a/engine/sim/sim.hpp +++ b/engine/sim/sim.hpp @@ -82,6 +82,7 @@ struct sim_t : private sc_thread_t bool initialized; bool fixed_time; bool save_profiles; + bool save_profile_pre_init; // Save profiles after actor init, but before init_finished. Allows for dumping of expressions that do not validate. bool save_profile_with_actions; // When saving full profiles, include actions or not bool save_full_profile; // save the full profile instead of only active save_e flags bool default_actions; From 6dc635509e5aef2f01e175e73eacf0c229273dcf Mon Sep 17 00:00:00 2001 From: Kate Martin <51387586+renanthera@users.noreply.github.com> Date: Fri, 20 Jun 2025 00:16:20 -0600 Subject: [PATCH 2/4] [report] Rearrange `print_profiles()` to more easily support inserting statements into file. --- engine/report/reports.cpp | 92 +++++++++++++-------------------------- 1 file changed, 30 insertions(+), 62 deletions(-) diff --git a/engine/report/reports.cpp b/engine/report/reports.cpp index c9cbd9e5102..524570b5517 100644 --- a/engine/report/reports.cpp +++ b/engine/report/reports.cpp @@ -23,63 +23,43 @@ namespace report void print_profiles(sim_t* sim) { + if ( sim->save_profile_pre_init ) + fmt::print( "Profiles generated with the save_profile_pre_init option may exhibit strange behaviour!\n" ); + int k = 0; - for (unsigned int i = 0; i < sim->actor_list.size(); i++) + for ( unsigned int i = 0; i < sim->actor_list.size(); i++ ) { - player_t* p = sim->actor_list[i]; - if (p->is_pet()) + player_t* p = sim->actor_list[ i ]; + if ( p->is_pet() ) continue; - if (!report_helper::check_gear(*p, *sim)) - { + if ( !report_helper::check_gear( *p, *sim ) ) continue; - } + k++; - if (!p->report_information.save_gear_str.empty()) // Save gear - { - io::cfile file(p->report_information.save_gear_str, "w"); - if (!file) - { - sim->error("Unable to save gear profile {} for player {}s\n", p->report_information.save_gear_str, - p->name()); - } - else - { - std::string profile_str = p->create_profile(SAVE_GEAR); - fprintf(file, "%s", profile_str.c_str()); - } - } + auto profile_writer = [ & ]( std::string filename, std::string type_str, save_e save_type ) { + if ( filename.empty() ) + return; - if (!p->report_information.save_talents_str.empty()) // Save talents - { - io::cfile file(p->report_information.save_talents_str, "w"); - if (!file) - { - sim->errorf("Unable to save talents profile %s for player %s\n", - p->report_information.save_talents_str.c_str(), p->name()); - } - else + io::cfile file( filename, "w" ); + if ( file ) { - std::string profile_str = p->create_profile(SAVE_TALENTS); - fprintf(file, "%s", profile_str.c_str()); + std::string contents = p->create_profile( save_type ); + if ( sim->save_profile_pre_init ) + contents.insert( + 0, + "# Warning! Profiles generated with the save_profile_pre_init option may exhibit strange behaviour!\n" ); + fprintf( file, "%s", contents.c_str() ); + return; } - } - if (!p->report_information.save_actions_str.empty()) // Save actions - { - io::cfile file(p->report_information.save_actions_str, "w"); - if (!file) - { - sim->errorf("Unable to save actions profile %s for player %s\n", - p->report_information.save_actions_str.c_str(), p->name()); - } - else - { - std::string profile_str = p->create_profile(SAVE_ACTIONS); - fprintf(file, "%s", profile_str.c_str()); - } - } + sim->error( "Unable to save {} profile {} for player {}s\n", type_str, filename, p->name() ); + }; + + profile_writer( p->report_information.save_gear_str, "gear", SAVE_GEAR ); + profile_writer( p->report_information.save_talents_str, "talents", SAVE_TALENTS ); + profile_writer( p->report_information.save_actions_str, "actions", SAVE_ACTIONS ); std::string file_name = p->report_information.save_str; @@ -98,23 +78,11 @@ void print_profiles(sim_t* sim) file_name += ".simc"; } - if (file_name.empty()) - continue; - - io::cfile file(file_name, "w"); - if (!file) - { - sim->errorf("Unable to save profile %s for player %s\n", file_name.c_str(), p->name()); - continue; - } - unsigned save_type = SAVE_ALL; - if (!sim->save_profile_with_actions) - { - save_type &= ~(SAVE_ACTIONS); - } - std::string profile_str = p->create_profile(static_cast(save_type)); - fprintf(file, "%s", profile_str.c_str()); + if ( !sim->save_profile_with_actions ) + save_type &= ~( SAVE_ACTIONS ); + + profile_writer( file_name, "", static_cast( save_type ) ); } // Save overview file for Guild downloads From 3785842ba05d3646d93df49dd3e0f9cec27fe7ba Mon Sep 17 00:00:00 2001 From: Kate Martin <51387586+renanthera@users.noreply.github.com> Date: Fri, 20 Jun 2025 00:18:04 -0600 Subject: [PATCH 3/4] [player] Add `save_full_blizzard_apl` option to write all rules regardless of relevance to simc. Requires `save_profile_pre_init` sim opt to not exit on failed expression validation. --- engine/player/player.cpp | 28 ++++++++++++------- .../player_processed_report_information.hpp | 4 +-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/engine/player/player.cpp b/engine/player/player.cpp index 2c712449777..8f123394b2f 100644 --- a/engine/player/player.cpp +++ b/engine/player/player.cpp @@ -3531,6 +3531,7 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist bool is_duplicate_2; bool is_duplicate_3; bool is_modified; + bool full = report_information.save_full_blizzard_apl; // TODO: verify < vs <= and > vs >= on all condition types switch ( rule.condition_type ) { @@ -3547,17 +3548,17 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist // TODO: Are there any other types of passives to check here? // TODO: What happens when Blizzard uses an aura here like they did with Mind Flay: Insanity? } - return ""; // no check necessary because simc actions are filtered out if the spell is not known + return !full ? "" : fmt::format( "SPELL_LEARNED({})", v1 ); // no check necessary because simc actions are filtered out if the spell is not known case SPELL_ON_COOLDOWN: assert( v2 == 0 && v3 == 0 ); if ( v1 ) return fmt::format( "!cooldown.{}.ready", tokenize_spell( v1 ) ); - return ""; // no check necessary because simc actions are not ready unless their cooldown is ready + return !full ? "" : fmt::format( "SPELL_ON_COOLDOWN({})", v1 ); // no check necessary because simc actions are not ready unless their cooldown is ready case SPELL_OFF_COOLDOWN: assert( v2 == 0 && v3 == 0 ); if ( v1 ) return fmt::format( "cooldown.{}.ready", tokenize_spell( v1 ) ); - return ""; // no check necessary because simc actions are not ready unless their cooldown is ready + return !full ? "" : fmt::format( "SPELL_OFF_COOLDOWN({})", v1 ); // no check necessary because simc actions are not ready unless their cooldown is ready case TARGET_DISTANCE_LESS: assert( v2 == 0 && v3 == 0 ); return fmt::format( "target.distance<={}", v1 ); @@ -3565,9 +3566,11 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist assert( v2 == 0 && v3 == 0 ); return fmt::format( "target.distance>{}", v1 ); case HOSTILE_TARGET: + assert( v1 == 0 && v2 == 0 && v3 == 0 ); + return !full ? "" : "HOSTILE_TARGET"; case FRIENDLY_TARGET: assert( v1 == 0 && v2 == 0 && v3 == 0 ); - return ""; + return !full ? "" : "FRIENDLY_TARGET"; case HEALTH_PCT_GREATER: assert( v2 == 0 && v3 == 0 ); return fmt::format( "target.health.pct>={}", v1 ); @@ -3630,12 +3633,14 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist if ( expr_str.find( "dot." ) == 0 ) return fmt::format( "active_{}>={}", expr_str, v1 ); // TODO: > or >=? // TODO: support debuffs - throw std::runtime_error( "Debuffs are unsupported for assisted combat condition AURA_COUNT_NEAR_PLAYER_GREATER." ); + if ( !full ) + throw std::runtime_error( "Debuffs are unsupported for assisted combat condition AURA_COUNT_NEAR_PLAYER_GREATER." ); + return fmt::format( "AURA_COUNT_NEAR_PLAYER_GREATER({},{},{})", v1, v2, v3 ); case AFFORD_COST: assert( v2 == 0 && v3 == 0 ); if ( v1 ) return fmt::format( "action.{}.cost_affordable", tokenize_spell( v1 ) ); - return ""; // no check necessary because simc actions are not ready unless their cost is affordable + return !full ? "" : fmt::format( "AFFORD_COST({})", v1 ); // no check necessary because simc actions are not ready unless their cost is affordable case AURA_MISSING_TARGET: assert( v2 == 0 && v3 == 0 ); expr_str = aura_expr_from_spell_id( v1, false ); @@ -3653,7 +3658,7 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist assert( v3 == 0 ); expr_str = aura_expr_from_spell_id( v1, true ); // TODO: Are there any cases where these would be talents we should worry about? - if ( expr_str.find( "talent." ) == 0 ) + if ( expr_str.find( "talent." ) == 0 && !full ) throw std::runtime_error( fmt::format( "Talents are unsupported for assisted combat condition AURA_DURATION_PLAYER.", rule.condition_type ) ); return fmt::format( "{}.up&{}.remains<={:g}", expr_str, expr_str, v2 / 1000.0 ); case AURA_DURATION_TARGET: @@ -3764,7 +3769,9 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist if ( expr_str.find( "dot." ) == 0 ) return fmt::format( "active_{}<={}", expr_str, v1 ); // TODO: < or <=? // TODO: support debuffs - throw std::runtime_error( "Debuffs are unsupported for assisted combat condition AURA_COUNT_NEAR_PLAYER_LESS." ); + if ( !full ) + throw std::runtime_error( "Debuffs are unsupported for assisted combat condition AURA_COUNT_NEAR_PLAYER_LESS." ); + return fmt::format( "AURA_COUNT_NEAR_PLAYER_LESS({},{},{})", v1, v2, v3 ); case TARGET_AURA_APPLICATION_GREATER: assert( v3 == 0 ); expr_str = aura_expr_from_spell_id( v1, false ); @@ -3785,7 +3792,7 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist assert( v2 == 0 && v3 == 0 ); if ( v1 ) return fmt::format( "spell_targets.{}>0", tokenize_spell( v1 ) ); - return ""; // no check necessary because simc actions are not ready unless they have a target + return !full ? "" : fmt::format( "SPELL_IN_RANGE({})", v1 ); // no check necessary because simc actions are not ready unless they have a target case HAS_PET: assert( v1 == 0 && v2 == 0 && v3 == 0 ); return "pet.any.active"; @@ -3831,7 +3838,7 @@ parsed_assisted_combat_rule_t player_t::parse_assisted_combat_rule( const assist case COOLDOWN_ALLOW_CASTING_SUCCESS: assert( v1 == 0 && v2 == 0 && v3 == 0 ); // This is handled elsewhere, since it removes a default condition instead of adding a new one. - return ""; + return !full ? "" : "COOLDOWN_ALLOW_CASTING_SUCCESS"; case PLAYER_HEALTH_PCT_GREATER: assert( v2 == 0 && v3 == 0 ); return fmt::format( "health.pct>={}", v1 ); @@ -13024,6 +13031,7 @@ void player_t::create_options() add_option( opt_string( "save_gear", report_information.save_gear_str ) ); add_option( opt_string( "save_talents", report_information.save_talents_str ) ); add_option( opt_string( "save_actions", report_information.save_actions_str ) ); + add_option( opt_bool( "save_full_blizzard_apl", report_information.save_full_blizzard_apl ) ); add_option( opt_string( "comment", report_information.comment_str ) ); add_option( opt_bool( "bugs", bugs ) ); add_option( opt_func( "world_lag", parse_world_lag ) ); diff --git a/engine/player/player_processed_report_information.hpp b/engine/player/player_processed_report_information.hpp index a7e7fde30a3..cf335300152 100644 --- a/engine/player/player_processed_report_information.hpp +++ b/engine/player/player_processed_report_information.hpp @@ -22,9 +22,9 @@ struct player_processed_report_information_t std::string save_gear_str; std::string save_talents_str; std::string save_actions_str; + bool save_full_blizzard_apl; std::string comment_str; std::string thumbnail_url; std::string html_profile_str; std::vector buff_list, dynamic_buffs, constant_buffs; - -}; \ No newline at end of file +}; From a4061d82049bf4130598d7a54ba126598cbf121a Mon Sep 17 00:00:00 2001 From: Kate Martin <51387586+renanthera@users.noreply.github.com> Date: Fri, 20 Jun 2025 02:01:09 -0600 Subject: [PATCH 4/4] [player] Narrow scope of changes and combine into a single player opt as `save_full_blizzard_apl` depended on `save_profile_pre_init` and the latter wasn't clearly independently useful. Narrow scope of what portions of code get skipped to just action expr parsing. --- engine/action/action.cpp | 8 ++-- engine/report/reports.cpp | 92 ++++++++++++++++++++++++++------------- engine/sc_main.cpp | 4 +- engine/sim/sim.cpp | 6 +-- engine/sim/sim.hpp | 1 - 5 files changed, 69 insertions(+), 42 deletions(-) diff --git a/engine/action/action.cpp b/engine/action/action.cpp index c58e092557f..9a10b8d2f32 100644 --- a/engine/action/action.cpp +++ b/engine/action/action.cpp @@ -2917,7 +2917,7 @@ void action_t::init_finished() player->dynamic_target_action_list.insert( this ); } - if ( !option.if_expr_str.empty() ) + if ( !option.if_expr_str.empty() && !player->report_information.save_full_blizzard_apl ) { if_expr = expr_t::parse( this, option.if_expr_str, sim->optimize_expressions ); if ( if_expr == nullptr ) @@ -2926,7 +2926,7 @@ void action_t::init_finished() } } - if ( !option.interrupt_if_expr_str.empty() ) + if ( !option.interrupt_if_expr_str.empty() && !player->report_information.save_full_blizzard_apl ) { interrupt_if_expr = expr_t::parse( this, option.interrupt_if_expr_str, sim->optimize_expressions ); if ( !interrupt_if_expr ) @@ -2935,7 +2935,7 @@ void action_t::init_finished() } } - if ( !option.early_chain_if_expr_str.empty() ) + if ( !option.early_chain_if_expr_str.empty() && !player->report_information.save_full_blizzard_apl ) { early_chain_if_expr = expr_t::parse( this, option.early_chain_if_expr_str, sim->optimize_expressions ); if ( !early_chain_if_expr ) @@ -2944,7 +2944,7 @@ void action_t::init_finished() } } - if ( !option.cancel_if_expr_str.empty() ) + if ( !option.cancel_if_expr_str.empty() && !player->report_information.save_full_blizzard_apl ) { cancel_if_expr = expr_t::parse( this, option.cancel_if_expr_str, sim->optimize_expressions ); if ( !cancel_if_expr ) diff --git a/engine/report/reports.cpp b/engine/report/reports.cpp index 524570b5517..c9cbd9e5102 100644 --- a/engine/report/reports.cpp +++ b/engine/report/reports.cpp @@ -23,43 +23,63 @@ namespace report void print_profiles(sim_t* sim) { - if ( sim->save_profile_pre_init ) - fmt::print( "Profiles generated with the save_profile_pre_init option may exhibit strange behaviour!\n" ); - int k = 0; - for ( unsigned int i = 0; i < sim->actor_list.size(); i++ ) + for (unsigned int i = 0; i < sim->actor_list.size(); i++) { - player_t* p = sim->actor_list[ i ]; - if ( p->is_pet() ) + player_t* p = sim->actor_list[i]; + if (p->is_pet()) continue; - if ( !report_helper::check_gear( *p, *sim ) ) + if (!report_helper::check_gear(*p, *sim)) + { continue; - + } k++; - auto profile_writer = [ & ]( std::string filename, std::string type_str, save_e save_type ) { - if ( filename.empty() ) - return; - - io::cfile file( filename, "w" ); - if ( file ) + if (!p->report_information.save_gear_str.empty()) // Save gear + { + io::cfile file(p->report_information.save_gear_str, "w"); + if (!file) + { + sim->error("Unable to save gear profile {} for player {}s\n", p->report_information.save_gear_str, + p->name()); + } + else { - std::string contents = p->create_profile( save_type ); - if ( sim->save_profile_pre_init ) - contents.insert( - 0, - "# Warning! Profiles generated with the save_profile_pre_init option may exhibit strange behaviour!\n" ); - fprintf( file, "%s", contents.c_str() ); - return; + std::string profile_str = p->create_profile(SAVE_GEAR); + fprintf(file, "%s", profile_str.c_str()); } + } - sim->error( "Unable to save {} profile {} for player {}s\n", type_str, filename, p->name() ); - }; + if (!p->report_information.save_talents_str.empty()) // Save talents + { + io::cfile file(p->report_information.save_talents_str, "w"); + if (!file) + { + sim->errorf("Unable to save talents profile %s for player %s\n", + p->report_information.save_talents_str.c_str(), p->name()); + } + else + { + std::string profile_str = p->create_profile(SAVE_TALENTS); + fprintf(file, "%s", profile_str.c_str()); + } + } - profile_writer( p->report_information.save_gear_str, "gear", SAVE_GEAR ); - profile_writer( p->report_information.save_talents_str, "talents", SAVE_TALENTS ); - profile_writer( p->report_information.save_actions_str, "actions", SAVE_ACTIONS ); + if (!p->report_information.save_actions_str.empty()) // Save actions + { + io::cfile file(p->report_information.save_actions_str, "w"); + if (!file) + { + sim->errorf("Unable to save actions profile %s for player %s\n", + p->report_information.save_actions_str.c_str(), p->name()); + } + else + { + std::string profile_str = p->create_profile(SAVE_ACTIONS); + fprintf(file, "%s", profile_str.c_str()); + } + } std::string file_name = p->report_information.save_str; @@ -78,11 +98,23 @@ void print_profiles(sim_t* sim) file_name += ".simc"; } - unsigned save_type = SAVE_ALL; - if ( !sim->save_profile_with_actions ) - save_type &= ~( SAVE_ACTIONS ); + if (file_name.empty()) + continue; - profile_writer( file_name, "", static_cast( save_type ) ); + io::cfile file(file_name, "w"); + if (!file) + { + sim->errorf("Unable to save profile %s for player %s\n", file_name.c_str(), p->name()); + continue; + } + + unsigned save_type = SAVE_ALL; + if (!sim->save_profile_with_actions) + { + save_type &= ~(SAVE_ACTIONS); + } + std::string profile_str = p->create_profile(static_cast(save_type)); + fprintf(file, "%s", profile_str.c_str()); } // Save overview file for Guild downloads diff --git a/engine/sc_main.cpp b/engine/sc_main.cpp index 2e20e029f17..33a83e98797 100644 --- a/engine/sc_main.cpp +++ b/engine/sc_main.cpp @@ -147,14 +147,14 @@ sim_t* sim_signal_handler_t::global_sim = nullptr; bool need_to_save_profiles( sim_t* sim ) { - if ( sim->save_profiles || sim->save_profile_pre_init ) + if ( sim->save_profiles ) { return true; } for ( auto& player : sim->player_list ) { - if ( !player->report_information.save_str.empty() ) + if ( !player->report_information.save_str.empty() || player->report_information.save_full_blizzard_apl ) { return true; } diff --git a/engine/sim/sim.cpp b/engine/sim/sim.cpp index 593d3544c61..f5df6d92e31 100644 --- a/engine/sim/sim.cpp +++ b/engine/sim/sim.cpp @@ -1397,7 +1397,6 @@ sim_t::sim_t() initialized( false ), fixed_time( true ), save_profiles( false ), - save_profile_pre_init( false ), save_profile_with_actions( true ), save_full_profile( true ), default_actions( false ), @@ -2901,9 +2900,7 @@ void sim_t::init() // We are committed to simulating something. Tell actors that the sim init is now complete if they // need to do something. - // Do not call init_finished if saving profiles without validation, or program - // possibly exits due to invalid expr exception. - if ( ! canceled && !save_profile_pre_init ) + if ( ! canceled ) { bool verify_use_items_state = true; @@ -3721,7 +3718,6 @@ void sim_t::create_options() add_option( opt_timespan( "ignite_sampling_delta", ignite_sampling_delta ) ); // Output add_option( opt_bool( "save_profiles", save_profiles ) ); - add_option( opt_bool( "save_profile_pre_init", save_profile_pre_init ) ); add_option( opt_bool( "save_profile_with_actions", save_profile_with_actions ) ); add_option( opt_bool( "save_full_profile", save_full_profile ) ); add_option( opt_bool( "default_actions", default_actions ) ); diff --git a/engine/sim/sim.hpp b/engine/sim/sim.hpp index d295168b081..cec5bfb2a05 100644 --- a/engine/sim/sim.hpp +++ b/engine/sim/sim.hpp @@ -82,7 +82,6 @@ struct sim_t : private sc_thread_t bool initialized; bool fixed_time; bool save_profiles; - bool save_profile_pre_init; // Save profiles after actor init, but before init_finished. Allows for dumping of expressions that do not validate. bool save_profile_with_actions; // When saving full profiles, include actions or not bool save_full_profile; // save the full profile instead of only active save_e flags bool default_actions;