search

Aldriana / ShadowCraft-Engine

Calculations backend for ShadowCraft, a WoW theorycraft project.
GNU Lesser General Public License v3.0
37 stars 22 forks source link

Talent Point Comparison #59

Closed Aldriana closed 13 years ago

Aldriana commented 13 years ago

I can see it being useful to have a function that computes the value of each talent point for a given spec. That is: something that goes through and adds/subtracts one point from each talent to estimate the DPS gain for each talent and returns a (sorted?) list of talents from best to worst. Note that the current restraints on the talent trees force there to be 31 points in one tree and less than 41 total, which makes this a bit tricky; that may need to be refactored to let this work.

dazer commented 13 years ago

I'm working on this. Calling self.talents.trees[tree].set_talent(talent, new_talent_value), tree being a value in (0, 1, 2), bypasses the checks so that should be easy. The talents classes have no notion of talent tiers, hence it renders impossible knowing what talents would be desirable to ckeck: it would compute the ranking for, say, slaughter from the shadows even if the spec we are analyzing is assassination; would it be good to call ep_for_talents() passing every talent that we want to know the ranking/EP for? Also, should the ranking attempt to come up with an EP value for talent or rather a dps difference for talent point?

Aldriana commented 13 years ago

I'd say ideally the talent value function should be smart enough to figure out which spec you are and only check talents that are accessible to that spec - i.e., only the first two tiers of the other trees.

In terms of setting values: is there anything wrong with just calling self.talents.tree.talent_name = value?

I guess I was thinking of this in terms of: write some logic to figure out how many talent points you have in each point, and add or subtract one point from each talent as applicable, and compute the damage differences; this will require handling any InputNotModeledExceptions that are raised for mandatory talents. It will also require adjusting the logic that expects there to be at most 41 points assigned and at least 31 in one tree, as those may no longer hold true.

And yeah, reporting results in dps is fine. I don't see much point in doing it in EP.

Aldriana commented 13 years ago

Recent commits seem to address this pretty well, so closing.