Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Sim talents in bulk sim #4205

Merged
merged 4 commits into from
Jan 30, 2024
Merged

WIP: Sim talents in bulk sim #4205

merged 4 commits into from
Jan 30, 2024

Conversation

ToxicKevinFerm
Copy link
Contributor

@ToxicKevinFerm ToxicKevinFerm commented Jan 28, 2024

Sorry if I'm making a mess, first time contributing, looking for feedback.

I thought about where to put the talent sims. My use case would be for example simming two set bonuses against eachother, but the set bonus requires a talent swap to maximise the dps.

So for now I've put it in the loop where we add the valid sims. There's still some issues, mainly in the GUI and we sim unnecessary talent loadouts right now. We shouldn't have to sim the currently loaded talent spec even if you pick it, because the bulksim does that automatically.

I think it's also important to note that this obviously multiplies the amount of sims that are needed for a result and so it only does the sims when activated.

It's not very nicely visualised in the UI either, and you probably want to equip the talent loadout as well on the button press.

@zku
Copy link
Contributor

zku commented Jan 29, 2024

Thanks a lot for sending this! To safe some work patching the changes in, could you show a screenshot of how the settings look like, how different talent loadouts are specified, and how the result list from bulk looks like with talent loadouts enabled?

@ToxicKevinFerm
Copy link
Contributor Author

Screenshot_45
Screenshot_49
Screenshot_48
Screenshot_47
Screenshot_46
Here's some screenshots of different views @zku .
Basically your talent loadouts are the saved talents you define in the talent section. I haven't figured out if I should listen to some events to update the saved talents etc, or what. And the view when youre both simming items and talents at the same time could look better.

@ToxicKevinFerm
Copy link
Contributor Author

Screenshot_50
Updated styling slightly

sim/core/bulksim.go Outdated Show resolved Hide resolved
@zku
Copy link
Contributor

zku commented Jan 30, 2024

Just one comment above, otherwise looks good to me. Thanks again for working on this!

@zku zku merged commit 9b66dc4 into wowsims:master Jan 30, 2024
1 check 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