Aldriana / ShadowCraft-Engine

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

Multiple proc behaviour #89

Closed dazer closed 12 years ago

dazer commented 13 years ago

I believe this meets our needs, hence the pull request. As it currently stands no changes to the output were made (other than avalanche's ep showing in the default scripts).

I think it's fairly understandable but, just in case: every proc object instantiated has the ability to change its behaviour by updating the behaviour_toggle attribute. Behaviours are stored into a dict on init. The enchants 'for loop' does exactly the same the older enchant checks did: append a proc to a list; the only major change is that it duplicates (copy module) some of the procs to produce a second (or third) object that is modified accordingly to be further processed (appended to the proc damage list in the case of avalanche). I also had to change how attacks_per_second works for these damage procs: it was overwriting the value on every pass, so the final computation in damage_breakdown never received multiple values for every damage proc and would instead apply the latest item in attacks_per_second to every proc, thus not producing sensible outputs.

See that new developers (for other classes maybe) may want to specify different behaviours: Avalanche's trigger for the spell component is periodic damage but this may not be true for, say, hunters. This could be addressed by having the proc class take a **dict for every behaviour and let the developers keep adding behaviours. I opted out of this to not overcomplicate the code but, if such day comes, implementign this would be very easy.

Edit: turns out it doesn't look too bad. New and different behaviours can be easily included by expanding the dicts that hold them.

Note that I didn't include the Hurricane spell component (left in comments an easy hack) because we'd need to properly model the refresh mechanic for this to make any sense. Notably, if the proc is included as is (no refresh and it always produces a third stack), the off-hand hurricane ep may even be higher than landslide (especially so with the new 4set). Now, modeling the refresh mechanic doesn't sound too complicated (it would include computing uptime distribution). It could potentialy lead to proper modeling of proc sinergy, which is something formulators don't usually do; sounds like fun, but probabilities is something I'm not big on.

Is there anything else we want from this? do any of you see a better implementation? does anyone want to model the refresh mechanic for Hurricane?

dazer commented 13 years ago

I imagine there's a million ways to do this. For instance I had at some point __getattr__ distribute the behaviour attributes deppending on the behaviour_toggle (current_behaviour fitted better in that approach). I also wrote the ProcBehaviour class to steer away from the behaviour setter, but it'd be simply a class to store values, with nothing else whatsoever that I could think of; I also went ahead and profiled it to find out that it lowered a tiny bit the performance (I imagine instancing objects is somewhat expensive). I find this approach easy to understand and use by anyone not familiar with the engine. However, I don't really know what is the most pythonic way to handle it so feel free to criticize it and I'll try my best to make it look neat.