Skip to content

Conversation

@renanthera
Copy link
Member

@renanthera renanthera commented Oct 25, 2025

Derived from the concept found in #10597, but generalized to allow for custom implementation in class modules.

Significant outstanding issues:

  • Error with merging enemy action lists. (undiagnosed, likely due to cancel position)
  • Many relevant player methods are useless at current sim_controller_t::evaluate_* calls. Makes it impossible to validate on objects such as secondary stats, etc.
  • Is there a better way to identify if a sim_t instance is a profileset? Currently using parent != nullptr && profileset_enabled which is not spectacular. Current strategy seems satisfactory. Can remove all risk by causing the relevant sim controller options to only be instantiated on profilesets. The inverse of this behaviour exists already (skipping profileset opt parsing for profileset child sims).

TODO:

  • Current return value of sim_controller_t::set_data<T>(...) is stupid. Probably should just be void, as a sim_controller_t::get_data<T>(...) for an improperly initialized derived sim_controller_t is an error which already asserts.
  • Using sim_t::cancel() does not clearly report what is occurring when a profileset gets eliminated.
  • std::shared_ptr<sim_controller_t> is not a good fit.
  • Implement error-based sim controller.
  • Implement system to allow options to instantiate sim controllers.
  • Simplify reporting methods, hook up to json and html outputs.
  • Cherrypick from TeWaWi to Midnight.

Developer interface issues:

  • Every derived sim_controller_t requires a using data_t = <derived from sim_controller_data_t; type alias. Every controller probably should get and set a derived sim_controller_data_t, just for the sake of reporting.
  • Using structured binding on the return value of sim_controller_t::get_data() is debatably a footgun. It's relatively uncommon in the simc codebase, but if the lock is not captured (which is prohibited as of time of writing via private), the temporary's lifespan will lapse prior to the end of the evaluate_controller_* method returning. This is very bad.
  • sim_controller_t::get_data<T>() and sim_controller_t::set_data<T>(...) require a template parameter which is always equal to the type-aliased data_t for the derived sim_controller_t. This kind of sucks, but I'm not sure of a way to avoid this without introducing manual mutex management and gratuitous casting.
  • Return value semantics of sim_controller_t::evaluate_* methods is somewhat awkward, but I haven't convinced myself if one is better than the other. ("keep going" => true, "keep going" => false) Settled on "keep going" => true.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant