Aldriana / ShadowCraft-Engine

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

PPM proc initialization #47

Closed Aldriana closed 13 years ago

Aldriana commented 13 years ago

Procs List, at current, doesn't seem to have any good way of handling PPM procs - it just builds procs out of the tuples in allowed_procs. If, for instance, DMC: Hurricane turns out to be 1 PPM (which would surprise me basically not at all), how would we properly initialize that?

As such, it seems like some minor refactoring may be needed.

dazer commented 13 years ago

Would set a new 'ppm' field in the tuples be enough? It would store the ppm if the proc is ppm or False if it's not. The PPMProc class is rendered useless since is_ppm can return Booleans checking the new field and proc_rate is now computed in the modeler side.

I think that what I don't understand is why are we subclassing ppm if it doesn't hold any usefull new method.

Aldriana commented 13 years ago

To be honest, I kind of forgot that we'd defined a proc_rate() function for PPM procs when I was writing the modeling - it'd probably be better to use the existing function. And even if it is going to be done modeler side, I think there's value in distinguishing between the variables proc_rate and ppm. I suppose one solution would be to just define both for all procs - I don't know how well that would actually work in practice.

As an aside, I'm almost not totally fond of the way we're instantiating procs in general. When you want to look up what a proc does, having to pick through an 8-tuple and figure out what field is which is not the most convenient thing in the world. It seems like there might be value in refactoring the information to be a little more clear on what's what. I don't have any particularly good ideas on how to do that, but its something to think about.

julienp commented 13 years ago

We could just put the proc data in dictionaries and make Proc and PPMProc take one of those dictionaries in init. Edit: having the data like this 'heroic_grace_of_the_herald': {'stat': 'crit', 'value': 1710, 'duration': 10, 'proc_change': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Herald of Doom'}, would make it easier to see what's what at a glance.

dazer commented 13 years ago

I'm not sure how that'd work, but I'm leaving here the data parsed to that way of dispaying it if anyone is interested in following: 'heroic_grace_of_the_herald': {'stat': 'crit', 'value': 1710, 'duration': 10, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Herald of Doom'}, # ICD is a guess and should be verified. 'heroic_key_to_the_endless_chamber': {'stat': 'agi', 'value': 1710, 'duration': 15, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 75, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Final Key'}, 'heroic_left_eye_of_rajh': {'stat': 'agi', 'value': 1710, 'duration': 10, 'proc_chance': .3, 'trigger': 'all_attacks', 'icd': 50, 'max_stacks': 1, 'on_crit': True, 'proc_name': 'Eye of Vengeance'}, # ICD is a guess and should be verified. 'heroic_prestors_talisman_of_machination': {'stat': 'haste', 'value': 2178, 'duration': 15, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 75, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Nefarious Plot'}, 'heroic_tias_grace': {'stat': 'agi', 'value': 34, 'duration': 15, 'proc_chance': None, 'trigger': 'all_attacks', 'icd': None, 'max_stacks': 10, 'on_crit': False, 'proc_name': 'Grace'}, 'darkmoon_card_hurricane': {'stat': 'spell_damage', 'value': 5000, 'duration': 0, 'proc_chance': None, 'trigger': 'all_attacks', 'icd': None, 'max_stacks': 0, 'on_crit': False, 'proc_name': 'Lightning Strike'}, # Behavior still needs to be tested. I expect 1 PPM with no ICD, but should be tested. 'essence_of_the_cyclone': {'stat': 'crit', 'value': 1926, 'duration': 10, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Twisted'}, # ICD is a guess and should be verified.
'fluid_death': {'stat': 'agi', 'value': 38, 'duration': 15, 'proc_chance': 1, 'trigger': 'all_attacks', 'icd': None, 'max_stacks': 10, 'on_crit': False, 'proc_name': 'River of Death'}, 'grace_of_the_herald': {'stat': 'crit', 'value': 924, 'duration': 10, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Herald of Doom'}, # ICD is a guess and should be verified. 'heart_of_the_vile': {'stat': 'crit', 'value': 924, 'duration': 10, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Herald of Doom'}, # ICD is a guess and should be verified. 'key_to_the_endless_chamber': {'stat': 'agi', 'value': 1290, 'duration': 15, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 75, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Final Key'}, 'left_eye_of_rajh': {'stat': 'agi', 'value': 1512, 'duration': 10, 'proc_chance': .3, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': True, 'proc_name': 'Eye of Vengeance'}, # ICD is a guess and should be verified. 'prestors_talisman_of_machination': {'stat': 'haste', 'value': 1926, 'duration': 15, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 75, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Nefarious Plot'}, 'rogue_t11_4pc': {'stat': 'weird_proc', 'value': 1, 'duration': 15, 'proc_chance': .01, 'trigger': 'auto_attacks', 'icd': None, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Deadly Scheme'}, 'the_twilight_blade': {'stat': 'crit', 'value': 185, 'duration': 10, 'proc_chance': None, 'trigger': 'all_attacks', 'icd': None, 'max_stacks': 3, 'on_crit': False, 'proc_name': 'The Deepest Night'}, # Behavior still needs to be tested. I expect 1 PPM with no ICD, but should be tested. 'tias_grace': {'stat': 'agi', 'value': 30, 'duration': 15, 'proc_chance': None, 'trigger': 'all_attacks', 'icd': None, 'max_stacks': 10, 'on_crit': False, 'proc_name': 'Grace'}, 'unheeded_warning': {'stat': 'weird_proc', 'value': .25, 'duration': 10, 'proc_chance': .1, 'trigger': 'all_attacks', 'icd': 45, 'max_stacks': 1, 'on_crit': False, 'proc_name': 'Heedless Carnage'} # ICD is a guess and should be verified.

Aldriana commented 13 years ago

General tip: if you have a dictionary:

 dict = {'a':1, 'b':2, 'c':3}

And you have a function/constructor/whatever called "foo", you can call

 foo(**dict)

Which is equivalent to

 foo(a=1, b=2, c=3)

Which you can thus use to pass the dictionaries of info into the functions.

Aldriana commented 13 years ago

I believe this was taken care of by the last round of proc refactoring, so closing.