aldriana / shadowcraft-engine Goto Github PK

Quoted:

I was thinking about it today, and I think we need to tweak... something, somewhere... to more easily enable EP calculations; there's a few that you can't easily compute by just externally toggling a stat on and off, notably hit and expertise cap related stuff. So the calculations... somewhere... need to be tweaked to allow EP values for yellow hit, spell hit, white hit, and expertise to be generated by manually hacking the right computed values to be the right amount higher, even if that doesn't exactly correspond to a real hit/expertise value.

I hacked precision into calcs.init in order to allow testing/debugging to work with the talent in place. However, this should really live in calcs.rogue.init or some such place instead, as it is rogue-specific. If someone could look at refactoring this I'd appreciate it; it not I'll get around to doing it at some point.

So, right now we have EP values for the basic gear stats; however, for gear comparisons its sometimes useful to assign EP values to other stats, such as: weapon speed, weapon damage, trinket procs, passive bonuses (i.e. metagem bonuses) and the like. Hence, we should think about adding a second EP function to calculate these slightly more obscure quantities.

To be clear: I think the existing function should stay with what it has, and we should add this new function in addition to that.

There was a fair amount of talk, and a little design, targeted at allowing this tool to work for multiple levels. The problem with that is the vast quantity of magic numbers that are now in the code. It does seem almost inescapable in a damage calculator like this - but it could be done with constant dictionarys or the like (ie the conversion constants in stats.py). As we continue to expand the tool it will only get more time consuming and tedious to go back and change these things; now would be a good time to decide whether we really want to support multiple levels or not.

We need some way of dealing with procs that have more than one proc rate/behavior. The simple example of this is Avalanche, which procs off weapon-based attacks at 5 PPM but spell-based attacks at 15%. The harder example is Hurricane, which is similar but the spell proc has an ICD while the melee proc does not. If someone wants to think about an elegant way of handling this, that'd be great.

It would probably be good to have some manner of logging framework in place to help us debug this when it inevitably breaks (or even just if it starts to give people questionable results). As I recall the python logging framework is pretty good - so it's probably just a matter of putting some sample code for setting up handlers somewhere and then starting to log stuff. If there's something we want to do (in terms of standarized log format or whatever) that isn't well covered by the default, it might be necessary to create some simple wrappers (which would presumably want to go in core.logging or the like) and then use those instead.

Copy paste of the previous closed issue. I'm keeping this 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.

Not much to change just yet, but there're some trinkets already datamined (and discussed in EJ). I'm pullin a request with the easiest ones and opening discussion on how to model the others.

-Hungerer: When using an ability causes your Energy or Focus to fall below 20% of your maximum, you will gain 1532/1730 haste rating for 15 sec. This cannot occur more often than once per minute. (Name of the buff is 'Devour')

Can be coded as an on use effect

-Ancient Petrified Seed: Increases your Agility by 1277/1441 for 15 sec. (1 Min Cooldown)

Straightforward 'on use' trinket

-Ricket's Magnetic Fireball: Calls down a meteor, burning all enemies within the area for 400 to 442 total Fire damage. (3 Min Cooldown)

Not sure how to go around this: it's not a proc so it shoud be included as a gear buff, but that class only covers stat boosts as of now. The use is worth like 2-3EP (it may as well not be worth it to model) but the trinket itself comes with a big chunk of agi so some users may want to have it.

-Matrix Restabilizer: Your melee and ranged attacks have a chance to grant 1532/1730 critical strike rating, haste rating, or mastery rating, whichever is currently highest. (The description doesn't say, but the uncategorized spell puts the duration at 30'').

Not sure how to go around this: interaction with other trinkets can force the modeler to proc the 'wrong' stat if we take the highest averaged stat, especially if the gearset has two of the stats close enough. Properly modeling the possible uptime of the 3 procs would require some serious refactoring I'm afraid. This could be seen as a proc bahaviour akin to hurricane/avalanche, I remember trying to approach that particular issue with no real success

-2T12: Your melee critical strikes deal 6% additional damage as Fire over 4 sec. (name of the damage proc is 'Burning Wounds')

Not entirely sure how to go around this: treating it as a proc like darkmoon card hurricane with a 'variable' damage (computed in the modeler); or special casing it completely in the modeler with its own get_uptime. I've already put together a fairly awfull hack for the best case scenario, but it's likely that the buff will munch itself, hence a proper proc handling is due. Also, we'd need some testing but I reckon it's fair to asume the proc to tick for 3% for a total of two ticks.

-4T12: Your Tricks of the Trade ability also causes you to gain a 25% increase to one of your combat ratings at random for 30 sec. (Names of the buffs are: master of flames, future on fire, fiery devastation)

Updating the stats (*= 3.25 / 3) in the modeler along with the crit bonus from T11 (and a check for tricks) sounds good enough to me as a ballpark figure. However, I'm not sure how much of an EP variance could the proc expose if we take into account the interaction with other procs. Also, implementing a delay on tricks to fish for procs could be an option to further delvelope the model.

In particular: figure out how shadowcraft-ui is exporting data (I think its cjson at this point), and write something that will parse it out and build the objects we're expecting.

It looks like the 'regular' version of this trinket is showing as lower dps than the LFR version of it. (Which, obviously, doesn't make any sense as the agility in the upgraded version is a direct upgrade.) Indeed, as I equipped the LFR version in the ui and then switched to the 403 (non-heroic version) version, my dps dropped by about 1.5k.

Two issues here - format to hold a (potentially) large quantity of integers. If we want to design for extensibility to other classes and potentially other levels, the total set of stats could eventually get fairly large. An xml format makes sense to me, but I'm open to ideas. Doing it all in python means some heavily nested data structures that I think should be avoided.

Second - what are the 85 values currently?

In the interest of having a centralized hub to discuss or check up what needs to be done, I'll be updating this lists as things get checked in. If you want to discuss one of these topics in more detail, feel free to open a different issue. Note that all these only relate to making the whole thing work again; if you feel more comfortable by refining other aspects of the library, those are always welcomed too.

-Write and/or update the 3 new models. This is, of course, the biggest and most delicate task; we need to keep a very tight control over it as that's where the strength of any formulator lies.
-Update the proc and gear buff 'databases' with the new trinkets and tier sets. Also, it'd be nice if we had a way for the front-end to simply input the spell ids for those so it could pick up on them without the issues we had in the past (see lfr-WoU).
-Implement the new racials (also, fix the exp/hit related ones).
-Figure out the formulae for shadow blades, poisons (dp and it's instant proc, wp and venomous wounds), dispatch, etc. Also: check that the old ones work as expected. This may need a bit of testing, but perhaps someone can figure this out browsing the dbc files (in simcraft/rawr/enhsim perhaps).
-New feature that would be extremely nice to have: a way to compute the compound ep of a weapon, so we don't have users swapping them like we did with the legendary daggers. Combat rogues will get to choose what type of mh weapon to weild so that'd be a good improvement.

Solved Issues:
-Talent and glyph rankings.
-Update the way mastery works if it's changed (ie: the paperdoll now shows it in the same fashion as other rating stats; how does that relate to the final percentage number that we need?)
-Have a way to internally implement the level-scaling tradeskill perks without forcing the front-end to tell what buff to use. Implemented those that are not linked to gear stats, like ring enchants and the like.
-Include and model the 3 new enchants, and how that relates to the proc class. Particularly, finish the way hurricane was handled or update the proc class so it can handle multiple-stats procs more nicely. Using the most p to date known behaviour.
-Check in the new miss/dodge/parry/glance mechanics (base rates and implications in the hit_chance methods).
-See how the new hit/exp propagate through the EP methods and update accordingly.
-Figure out a better way to tell the modeler what cycle (spec) to choose (the new talents are not tied to the spec).

objects/stats.py

In activated_boosts, Ancient Petrified Seed is listed as a 1227 Agi on-use effect. Should be 1277.

I think it's time we start thinking about the next xpack. Before I start pushing code I'd like to know what people think should be done. I'll throw some thoughts out, see what you guys think:

For starters, how do these big changes affect us? would it be interesting to keep somehow the present Shadowcraft as a comparison tool? should we keep adding to the existing project?, start a new one?, branch the existing one as to signify a milestone?.

Talents got a major revamp. I imagine we don't need the subclassing anymore as game-classes are a lot more simple in this department. Do we want a similar input like we had before with the three numeric strings? it'd be something like '121132'; or keep the format for glyphs (passing all the talent names)?.

The damage calculator class needs some clean-up since talents, as we once knew them, are gone. I recall some formulas were coded somewhat funky so that'll need some work too. As for numeric values, once it's possible we'll need to figure the new damage modifiers, combat ratings, armor, etc. In the same line, the model will need some major updates to include the new abilities and deprecate old talents.

Frontend: I don't know if Antiarc will still be doing his amazing work; the last updates got a bit slower than I would have liked (with all the confusion in the forums). I've been toying with Android and I intend to make one app for ShadowCraft at some point in the future (porting the engine to Java and all that). Did the python UI get any use? I kept adding items but didn't really play much with it.

Other classes: I might try and write a model for windwalkers. Is this to be merged with Aldriana's project? or should I keep my own repo? is anyone else interested in working on other specs or helping out with monks?.

It occurred to me that it could be usefull if the functions in RogueDamageCalculator returned both the damage and the crit damage: if we are to output the breakdown we may as well provide info on how much our abilities are critting for (that was the thinking behind low and high end damage for evis/IP).
In 3.x spreadsheets this was handled through a modifier that included both the crit chance and the crit bonus. As neat as I always though that was it may be better suited to keep all the damage formulas together now, then compute crit chances along with cycles (since you need crit chances there) and send both further down the pipe into the final dps/ep calculator.
If I ever figure out how to update my fork to the new commit(s) made, I'll start writing crit damage, also including things like lethality, meta gem, sanginary vein, arcane tactics, mangle, savage combat and master poisoner. I'll give some thought to active buffs (RvS, KsP, Vendetta and Find Weakness from the top of my head).

In combat cycles, the dps breakdown function will throw different data (for all items but killing spree) if called after talent_ranking(). This behaviour won't happen if we don't use the talent cache; I'll be digging around the issue but, for the time being, I'm disabling the cache in my forks until we trace the bug; see that this change significantly decreases performance.

Quoted:
There's some sort of data-entry-ish tasks around putting the rest of the talents and glyphs in. Not terribly hard, but needs to be done at some point.

Quoted:
At some point the calcs.rogue.RogueDamageCalculator level functions need to be built out - in the short term, damage functions in terms of AP and mastery for Envenom, Rupture, Venomous Wounds, IP, and DP would be useful, and eventually we'll want Evis, SS, RvS, Hemo, Main Gauche, etc. as well. Even an autoattack damage function isn't inherently ridiculous. Mastery detection functions akin to the Assassin's Resolve one seems like a good idea. There are probably others as well, but those are the first handful that spring to mind.

This should be done at some point. Suggested method involves gettext, but if you have a better way - let us know.

Do you want to deal with this fully? From glancing at the current clalcs/init.py this would mean separating main-hand and off-hand melee_hit_chance and adding a more specific weapon type to Weapon. I don't know how much complication this offers to the rogue-specific calcs you're working on.

The races.py I've just pushed will supply the expertise value (or crit chance mod if it's ranged) for these racials.

Noticed this while trying to add some unittests, there's a "stats" missing here: https://github.com/Aldriana/ShadowCraft-Engine/blob/master/calcs/init.py#L134, should be
hit_chance = 1 - max(self.BASE_SPELL_MISS_RATE - self.stats.get_spell_hit_from_rating(),0)

https://github.com/Aldriana/ShadowCraft-Engine/blob/master/calcs/rogue/Aldriana/init.py#L74 should be 25 instead of 24

It's set to 2880 but should be 1920.

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.

Might be nice to have the ability to generate one-handed expertise values - i.e., a orc or gnome wielding axe/dagger, above the racial expertise cap but below the overall cap. Probably don't want to compute this all the time, but having the ability to get it when needed would be nice.

For that matter, we might want to think about defining an EP function where you only get EP values for the base stats you care about. For instance, if at some point a caster model is built, they'll want int and spirit EP values, but rogues don't need those. Or, in most cases, to parry expertise calculations. This probably wants to be user configurable, but have a sensible default.

It seems like most of the racials can be tied into the existing framework(s) we have; for instance, we could have the Worgen racial be detected automatically in the crit calculations, create a wrapper function in the calculator that pulls in both gear buffs and activated racial abilities so they can be iterated through in a single call, and so forth. Basically: we're already handling the expertise racials automatically without the modeler needing to do anything; it would be good to see how many of the others we can do the same way.

Not much to change just yet, but there're some trinkets already datamined (and discussed in EJ). I'm pullin a request with the easiest ones and opening discussion on how to model the others.

-Hungerer: When using an ability causes your Energy or Focus to fall below 20% of your maximum, you will gain 1532/1730 haste rating for 15 sec. This cannot occur more often than once per minute. (Name of the buff is 'Devour')

Can be coded as an on use effect

-Ancient Petrified Seed: Increases your Agility by 1277/1441 for 15 sec. (1 Min Cooldown)

Straightforward 'on use' trinket

-Ricket's Magnetic Fireball: Calls down a meteor, burning all enemies within the area for 400 to 442 total Fire damage. (3 Min Cooldown)

Not sure how to go around this: it's not a proc so it shoud be included as a gear buff, but that class only covers stat boosts as of now. The use is worth like 2-3EP (it may as well not be worth it to model) but the trinket itself comes with a big chunk of agi so some users may want to have it.

-Matrix Restabilizer: Your melee and ranged attacks have a chance to grant 1532/1730 critical strike rating, haste rating, or mastery rating, whichever is currently highest. (The description doesn't say, but the uncategorized spell puts the duration at 30'').

Not sure how to go around this: interaction with other trinkets can force the modeler to proc the 'wrong' stat if we take the highest averaged stat, especially if the gearset has two of the stats close enough. Properly modeling the possible uptime of the 3 procs would require some serious refactoring I'm afraid. This could be seen as a proc bahaviour akin to hurricane/avalanche, I remember trying to approach that particular issue with no real success

-2T12: Your melee critical strikes deal 6% additional damage as Fire over 4 sec. (name of the damage proc is 'Burning Wounds')

Not entirely sure how to go around this: treating it as a proc like darkmoon card hurricane with a 'variable' damage (computed in the modeler); or special casing it completely in the modeler with its own get_uptime. I've already put together a fairly awfull hack for the best case scenario, but it's likely that the buff will munch itself, hence a proper proc handling is due. Also, we'd need some testing but I reckon it's fair to asume the proc to tick for 1.5% each second.

-4T12: Your Tricks of the Trade ability also causes you to gain a 25% increase to one of your combat ratings at random for 30 sec. (Names of the buffs are: master of flames, future on fire, fiery devastation)

Updating the stats (*= 3.25 / 3) in the modeler along with the crit bonus from T11 (and a check for tricks) sounds good enough to me as a ballpark figure. However, I'm not sure how much of an EP variance could the proc expose if we take into account the interaction with other procs. Also, implementing a delay on tricks to fish for procs could be an option to further delvelope the model.

Missing an 'ap' argument here:
https://github.com/Aldriana/ShadowCraft-Engine/blob/master/calcs/rogue/init.py#L217
Should be def hemorrhage_damage(self, ap, armor=None):

You could probably also rename main_gauche to main_gauche_damage in the same file.

It appears that the 4pc T12 bonus is not being applied to trinket procs. Is this intentional, or an oversight?

(To be clear: its simple enough to change the behavior, I just want to make sure I didn't miss testing somewhere that indicates that it shouldn't. Because my immediate instinct is to say that it should.)

https://github.com/Aldriana/ShadowCraft-Engine/blob/master/calcs/init.py#L98 should be parry_chance with only one underscore.

Restless Blade is not correctly implemented for Killing spree, but for adrenaline rush it's correct:

Line 1091: ksp_cooldown = 120 / total_restless_blades_benefit + self.settings.response_time

should by:

ksp_cooldown = 120 / (1 + total_restless_blades_benefit) + self.settings.response_time

And also at the point, where we should wait for bandit's guile to be red:

Line 1101: avg_wait_till_full_stack = 1.5 * time_at_level / cycle_duration

should by some

avg_wait_till_full_stack = 1.5 * time_at_level

After these fixes Killing Spree immediatly is better then wait for red phase.
Please look over it, so you can verify that my corrections are correct. :)

Sorry for bad english

Eventually we plan to hook this up with Antiarc's Shadowcraft-UI project for UI/frontend stuff. However, it seems like there may be some merit to writing some manner of primitive frontend for development and testing purposes. Basically: while its all well and good to operate through the UI for most purposes, I can see it being hard to distinguish front-end bugs from back-end bugs when operating through that UI.

Hence: it seems to me that having some manner of basic frontend - ideally pure python (so you don't have to install anything else to run it) to allow basic access to gear, gems, and reforging in a way that doesn't require manually adding up stats in a separate spreadsheet.

In short: I'm thinking about something about as functional as the front page of my previous spreadsheets - something that'll add up stats and give you a DPS estimate + EP values and tell you how you're doing relative to the various caps... and not a whole lot more. Does such a tool make sense to people, and does anyone feel like writing it?

Quoted from another message:
Going through and putting legitimate error messages instead of bare asserts is probably a good idea. Seems to me that we should define an InvalidInputsException (or something like that) and make a point to mostly raise those, so calling programs can easily distinguish between those and stuff breaking. Also note that i18n-ing the error messages in those exceptions so they can be displayed directly would probably make sense.

We don't have professions anywhere. They probably should be passed as a list of strings to Stats.

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.

The damage functions in RogueDamageCalculator don't seem to include Killing Spree. If would be handy if that could be added.

A real unit test framework would be good. Pyunit is something to look into. The current test.py isn't very robust - a complete regression suite would be ideal.

Currently the project root is not a proper python package, it could be useful if we made a package that could be installed with distribute and pip/easy_install.

We'd have to move the packages into a base package called shadowcraft

ShadowCraft-Engine (git root dir)
    - scripts
        - assassination.py
        - combat.py
        - subtlety.py
    - shadowcraft
        - calcs
        - core
        - objects
    - tests
        ...
    - license.txt
    - README
    - setup.py
    - style.txt

Imports would have to be changed to import from the new shadowcraft base package, from shadowcraft.objects import buffs etc.

I did the changes to the package structure and imports in a test branch and all tests pass. There could be some merge issues if someone got changes that are not in the current master branch, so it would be best to get all those in before doing it.

So the change wouldn't be difficult, but I'm not entirely sure we need it. I don't have a direct use for it, Shadowcraft-UI might find it useful, but possibly it's unneeded. If it turns out to be needed sometime in the future, getting it done early is probably good. What are people's thoughts on making it a real (and installable) python package?

2 more requests for procs handling:

1. Expose an "only procs on crits" type variable so callers can access that information without manually having to check proc.trigger. This also will require adding the crit-only triggers to the existing booleans.

2. Store the name of the proc somewhere in the proc object. This has two purposes. First, for damage procs, it allows us to include the correct damage name in the damage dictionary (i.e., for Tiny Abom we'd like to be able to report a damage total for "manifest_anger"). Second, it allows debugging type information around the uptime of procs to be returned - it's helpful to be able to drill into the damage calculator object and figure out what sort of uptime you should expect on Lighting Speed and the like.

Copy paste of the previous closed issue:

Not much to change just yet, but there're some trinkets already datamined (and discussed in EJ). I'm pullin a request with the easiest ones and opening discussion on how to model the others.

-Hungerer: When using an ability causes your Energy or Focus to fall below 20% of your maximum, you will gain 1532/1730 haste rating for 15 sec. This cannot occur more often than once per minute. (Name of the buff is 'Devour')

Can be coded as an on use effect

-Ancient Petrified Seed: Increases your Agility by 1277/1441 for 15 sec. (1 Min Cooldown)

Straightforward 'on use' trinket

-Ricket's Magnetic Fireball: Calls down a meteor, burning all enemies within the area for 400 to 442 total Fire damage. (3 Min Cooldown)

Not sure how to go around this: it's not a proc so it shoud be included as a gear buff, but that class only covers stat boosts as of now. The use is worth like 2-3EP (it may as well not be worth it to model) but the trinket itself comes with a big chunk of agi so some users may want to have it.

-Matrix Restabilizer: Your melee and ranged attacks have a chance to grant 1532/1730 critical strike rating, haste rating, or mastery rating, whichever is currently highest. (The description doesn't say, but the uncategorized spell puts the duration at 30'').

Not sure how to go around this: interaction with other trinkets can force the modeler to proc the 'wrong' stat if we take the highest averaged stat, especially if the gearset has two of the stats close enough. Properly modeling the possible uptime of the 3 procs would require some serious refactoring I'm afraid. This could be seen as a proc bahaviour akin to hurricane/avalanche, I remember trying to approach that particular issue with no real success

-2T12: Your melee critical strikes deal 6% additional damage as Fire over 4 sec. (name of the damage proc is 'Burning Wounds')

Not entirely sure how to go around this: treating it as a proc like darkmoon card hurricane with a 'variable' damage (computed in the modeler); or special casing it completely in the modeler with its own get_uptime. I've already put together a fairly awfull hack for the best case scenario, but it's likely that the buff will munch itself, hence a proper proc handling is due. Also, we'd need some testing but I reckon it's fair to asume the proc to tick for 1.5% each second.

-4T12: Your Tricks of the Trade ability also causes you to gain a 25% increase to one of your combat ratings at random for 30 sec. (Names of the buffs are: master of flames, future on fire, fiery devastation)

Updating the stats (*= 3.25 / 3) in the modeler along with the crit bonus from T11 (and a check for tricks) sounds good enough to me as a ballpark figure. However, I'm not sure how much of an EP variance could the proc expose if we take into account the interaction with other procs. Also, implementing a delay on tricks to fish for procs could be an option to further delvelope the model.

I think we want a little more detailed proc handling than we currently have - there's basically three things about the current setup that I'm finding a bit cumbersome to work with.

1. First and foremost, pulling stuff out of a five-tuple isn't the clearest and easiest way to do business. Keeping track of what entry is what requires a lot of flipping back and forth between sections of code - I think it'd be nicer if there was a proc object that would allow you to just read the relevant attributes off proc.value, prod.duration, proc.icd, and so forth.

2. There's a couple proc properties not well supported by the current framework - the notable ones that spring to mind are PPM and stacking procs (i.e Fluid Death). This could be fixed by adding still more things to the tuple (i.e., more named variables per point 1), but it might also make sense to subclass the proc object. Then you can have PPMProc (or whatever it winds up being called) take weapon speed as an argument to proc_rate() as opposed to just returning a static percentage.

Edit: I suppose only PPM really requires subclassing, as a regular proc can probably be handled as a stacking proc with max_stacks = 1.

3. The current triggering rules aren't really that useful for actually figuring out how many procs you're going to get. I think it'd be nicer if the proc exposed a set of booleans for each type of triggering effect, so you can call proc.procs_off_autoattacks and it'll return True or False. You probably wind up needing something like 5 different categories (autoattacks, strikes, debuff application (i.e. rupture), spells, and periodic damage), plus a toggle for crits only. I could do that classification in the model, I suppose, but I think it makes more sense to have proc object itself be smart enough to tell you those things.

So, basically, what I'm envisioning is: you have a set of Proc objects that define these sorts of useful quantities, and then the current Procs object is basically just a container for these that's smart enough to map a list of proc strings to the corresponding procs, and has methods to return all procs for a given stat. It's probably not ideal to have the classes be called Proc and Procs, but you get the idea.

I'm going to need this before I get too much further on the modeling front, so if you're interested in doing this please let me know - otherwise I'll probably work on implementing it sometime Sunday or Monday.

How do you picture this for the various activated racials, on-use trinkets, potions, etc? I don't mind digging around for values on these things, but I'm not sure what format to offer them in. For the moment I'm going to compile a dictionary of tuples that you can query for each piece of data, ie
activated_ability_data = {
'unsolvable_riddle': (1605, 20, 120)
}

Something similar would probably make sense for procs (value, proc chance, icd), but data on these is currently much less well-known.