Aldriana / ShadowCraft-Engine

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

4.2 update #81

Closed dazer closed 13 years ago

dazer commented 13 years ago

I mistakenly closed the request (and the issue) so here goes again. I'm keeping the issue open to discus what needs to be done and why (new features, or more trinkets, or changes to the class that get released). Will be commenting on how I do that in the pull request.

I included the response time in t12_4pc and updated the method a bit to have less nested 'if' logic.

The stat for Matrix is set at the start of the cycle iteration in set_constants. The call can be made in compute_damage (like unheaded warning), but I imagine under certain gearsets (which I haven't been able to design yet) it could be possible for the engine to enter an infinite loop through are_close_enough.

Ricket fireball is handled ala rocket barrage and included a method in stats to average it's damage value; I expect it to change somehow, based in AP damage or something in a future patch (othersise the proc would be certainly lackluster).

As for Hungerer, I'm leaving it as is untill you write the logic behind the trigger. If the cycle is executed properly the trinket will proc eventually, but I suspected users would try to expend energy as soon as it comes off cd and that's why I modeled like so.

The approach I took for t12_2pc seems a bit naïve to me, but I couln't find anything better (it's basicly a quick hack I wrote to be able to play with it a bit). I kind of reverse engineer the crit contribution directly from damage_breakdown. Also, the outcome is a bit too low for what is expected, so I may be doing something wrong. EDIT: nevermind this: I'm positive I'm doing somethign wrong; will update sometime this week.

dazer commented 13 years ago

Fixed t12_2pc. I computed the crit contribution for every ability and appended it to the damage breakdown dictionary so it now is a dictionary of tuples (damage, crit component). Note that this crit component gets deleted right after the tier set damage is calculated, but we could keep refactoring the methods down the line (spec_dps_breakdown) to make it easily available to the front end if needed.

dazer commented 13 years ago

I'm getting the following error when trying to load the testing suite with some of the new weapons (left those in comments in my most recent commint).

    self.dps.SetValue(str(dps))
UnboundLocalError: local variable 'dps' referenced before assignment
22:08:59: Debug: ..\..\include\wx/msw/private.h(697): 'UnregisterClass' failed w
ith error 0x00000584 (la clase todavía tiene ventanas abiertas.).

(Class Still Has Open Windows) which might be related to my winXP setup; just pointing it out so you can check if it works properly on your machine.

Included the new non crit behaviour of DMC:Hurricane.

dazer commented 13 years ago

I needed the previous updates by julienp and cheald relating to 4.06 and 4.1 (as well as some code optimization) so I pulled/merged them in my fork. I thoroughly revised them and, aside from the unheaded warning implementation(*), I find it very nicely done. If you find it better to pull each part separatedly I can redo my pull request after pulling theirs.

(*)unheeded warning: not sure whether the place and way it's coded is the best approach.

Also included the 4.2 buffs so that usefull EP weights can be pulled from the engine for the upcomming major patch. Will update Ricket's trinket once we figure it's behaviour.

dazer commented 13 years ago

Changed the envenom formula, which I wrote too hastly when describing the damage functions, to correctly reflect the dependency on dp_charges. Also, updated Sinister Calling to my most recent findings; note that this change puts subtlety modelling ahead of both combat and assassination!, which is a bit confusing to say the least.

dazer commented 13 years ago

I'm implementing the usage of hemo into the subtlety cycle. If the usage is set to 'never' the outcome is exactly the same as before; other settings reveal very consistent data; nevertheless, I'll spend some more time testing it. The implementation is nothing fancy but I think some explanation is due since Aldriana clearly stated that any change to his model would be thoroughly revised.

It simply checks for 'never' or 'always' and then uses the energy consumption accordingly. If the input is a number that'd be the interval between hemorrhages, which is used to compute a energy regen increase by replacing a set of backstabs for hemorrhages. There's a new check at the end to convert attacks_per_second[cp_builder] to the correct backstab and/or hemorrhage count. I was tempted to throw the response_time somewhere, but I don't think that's a necessity in this case.

I had to change the name of plenty of variables: those that included 'backstab' were changed for a more consistent 'cp_builder'. I also deleted references to 'energy_regen_with_recuperate' and similar ones since I was introducing a new 'regen_with_replaced_backstabs' and it was fairly easy to mess them all up, so I decided for a more neutral 'modified_energy_regen'; besides, that naming convention was more of a stub from previous versions than anything. I changed the name of 'backstab_time' for 'backstab_interval' because it was bugging me. While developing it I noticed that I personaly understand the whole thing a lot better when using frequencies rather than intervals but decided against it for the input makes more sense in intervals (hemorrhage every 24 or 60 seconds). I can, of course, reverse or change any of all that naming if needed.

I'm not completely content with the settings 'never', 'always' or a number but I guess that's the best approach. I started using numbers and booleans, where the number 0 could also represent the highest frequency (always use hemo that is) but it seemed a bit counterintuitive (and rather frustrating as infinite should/could represent 'never').

dazer commented 13 years ago

Despite the long description in the commit, last update was not much more than refactoring to use a single method holding damage on-use, including rocket barrage. While the new method is technicaly unnecesary, I think it's good to have it just in case.

The only modeled damage on-use (aside form goblin barrage) is the Magnetic Fireball trinket, which is actually working as a proc in the ptr now (trigger on every attack, I guess, as even sap can trigger it; 20% proc chance as seen in wowhead; icd: about two min). The crit on-use has no cooldown, you can keep the buff 100% uptime that is, so I still expect some changes to its behaviour (along with the seemingly unobtainable Aella's Bottle).

Ricket's was updated in wowhead last night and now has a CD on the crit buff. The hungerer got and update too and now works as a proc, dropping the energy/focus logic. I'm setting the icd to 5*duration but this, and its proc chance, should be verified.

dazer commented 13 years ago

I forgot to update the test suite, as usual. There are 24 errors all related to 12 weapon strike damage formulas: neither of them can figure a value for 'unheeded_warning_bonus' which is done in another module.

Edit: instead of changing the tests, I refactored the trinket. The tests pass and the value for the trinket remains exactly the same so I'm calling it good. Edit2: this change introduced a bug into the weapon dps EP formula. I'm reverting to the previous iteration and refactor it a bit so it's reusable and tests pass.

Aldriana commented 13 years ago

So, thanks for all the work on this - I'm glad people have been maintaining this while I haven't had time. Hopefully I'll have a little more time to poke at this over the next few weeks to clean some things up for Firelands.

Is this pull request in a place that y'all are happy with, or is there additional work that's been done that needs to be added?

dazer commented 13 years ago

I have some work done on the weapon enchant multiple behaviour, but I think that's worthy of it's own separate pull. We'd also need some final confirmation on the behaviour of the new trinkets (it's updated to your most recent findings and wowhead doesn't seem to have changed any); other than that, this is all I could think that needed to be done. If you find anything can be better written, or any kind of bugs, I'll could fix them without much problem.

Aldriana commented 13 years ago

I looked through it briefly and it looks broadly reasonable; this is not to say I might not find things in need of tweaking when I start hacking on things like Mutilate cycle improvements, but its in a place where I'm not averse to pulling it. Just wanted to check if there was anything you wanted to finish up before I pulled it; it appears the answer is no, so I'm going to go ahead and do that.