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

Standardize default fight length across sims #1143

Merged
merged 2 commits into from
Oct 30, 2024
Merged

Conversation

NerdEgghead
Copy link
Contributor

On branch unify-fight-length
Changes to be committed:
modified: ui/core/encounter.ts
modified: ui/core/individual_sim_ui.tsx
modified: ui/druid/feral/presets.ts
modified: ui/hunter/beast_mastery/presets.ts
modified: ui/hunter/marksmanship/presets.ts
modified: ui/hunter/survival/presets.ts
modified: ui/paladin/retribution/presets.ts
modified: ui/rogue/assassination/presets.ts
modified: ui/rogue/combat/presets.ts
modified: ui/rogue/subtlety/presets.ts
modified: ui/shaman/elemental/presets.ts
modified: ui/shaman/enhancement/presets.ts
modified: ui/warlock/affliction/presets.ts
modified: ui/warlock/demonology/presets.ts
modified: ui/warlock/destruction/presets.ts

 On branch unify-fight-length
 Changes to be committed:
	modified:   ui/core/encounter.ts
	modified:   ui/core/individual_sim_ui.tsx
	modified:   ui/druid/feral/presets.ts
	modified:   ui/hunter/beast_mastery/presets.ts
	modified:   ui/hunter/marksmanship/presets.ts
	modified:   ui/hunter/survival/presets.ts
	modified:   ui/paladin/retribution/presets.ts
	modified:   ui/rogue/assassination/presets.ts
	modified:   ui/rogue/combat/presets.ts
	modified:   ui/rogue/subtlety/presets.ts
	modified:   ui/shaman/elemental/presets.ts
	modified:   ui/shaman/enhancement/presets.ts
	modified:   ui/warlock/affliction/presets.ts
	modified:   ui/warlock/demonology/presets.ts
	modified:   ui/warlock/destruction/presets.ts
@InDebt
Copy link
Contributor

InDebt commented Oct 30, 2024

This would not affect any existing session any user has for Wowsims, right?
As encounter length is to my knowledge not tied to the t11 or t12 prests, users would only get this by 'Restoring defaults', right?

Is there a way we can use something similar tot he stat changes to 'force' update the default fight settings for this version once?

@Polynomix
Copy link
Contributor

For ele sims, you don't glyph the same for 240s fight and 360s fight. But i'm fine with this since i believe people who cares enough about it will run their own duration with the setup they want.

@NerdEgghead
Copy link
Contributor Author

This would not affect any existing session any user has for Wowsims, right? As encounter length is to my knowledge not tied to the t11 or t12 prests, users would only get this by 'Restoring defaults', right?

Is there a way we can use something similar tot he stat changes to 'force' update the default fight settings for this version once?

There are some hacky ways to force this, but I don't think it's a good idea personally. Changing people's stored settings without a prompt isn't good practice unless it's truly required for the sim to run properly (stats indexing changes etc.). If someone had a custom fight length they were optimizing for but hadn't saved it as a preset, a forced reset to defaults will be very annoying for that user. Better practice is to just recommend that people restore to defaults on class Discords etc. That's just my two cents on this anyhow.

If we move towards my "Option 2" idea, then what we could do is utilize the Preset Builds functionality that @1337LutZ put in (which I heavily use for Guardian), and make sure that every spec always has a Preset Build with just the default encounter + fight length recommendation for the current phase saved in it. Then there's a button on that tab which a user can click to load all encounter defaults without needing to go into the global settings menu and restore everything.

@NerdEgghead NerdEgghead merged commit 8fb08a2 into master Oct 30, 2024
2 checks passed
@NerdEgghead NerdEgghead deleted the unify-fight-length branch October 30, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants