Skip to content

Conversation

@renanthera
Copy link
Member

@renanthera renanthera commented Oct 21, 2025

Derived from the concept found in #10597, but generalized to allow for extension.

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.

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.
  • 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)

@pull pull bot force-pushed the thewarwithin branch from 24764ea to afcf0ba Compare October 22, 2025 00:57
@Saeldur Saeldur merged commit afcf0ba into simulationcraft:thewarwithin Oct 24, 2025
218 of 219 checks passed
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.

2 participants