Changing Level

[raconzor]

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.

[Aldriana]

I view level dependency as a feature that would be nice to have, but not an urgent priority - I think it has a lot more to do with the next expansion than this one. And once we pull the last couple of level-dependent constants into the general portions of the tool (base AP formula and glance rate) the problem isn't actually going to get any harder for a while. So I agree its a good feature to have, but I wouldn't prioritize it terribly highly for the moment.

[raconzor]

I would probably rate this last among the issues currently listed, but I can see a near-term use in verifying modeling methodology by running it at level 80 and comparing the results with existing tools/empirical data.

Either way, when this is done, we will have a very large collection of constants/constant dictionaries. Should these go in the classes they effect? I sort of support moving them to a separate python file to clean things up a bit, else we could easily have 20-30 lines at the top of several classes (DamageCalculator in particular).

[Aldriana]

20-30 lines doesn't honestly bother me that much. If it gets up to 50 or 100 we might need to reconsider, but for 20 lines I wouldn't do anything drastic.

That said: I also don't really care if they're pulled out into other files, provided its done in a way that makes the code paths clear and easy to follow.

[dazer]

What at we looking at here? huge sets of data? or do we have any formula that makes the correspondance? I happen to have the tuple for backstab; trying to infer the formulas I came to the conclusion that there are some breaking points (at 59 and 80) but I couldn't figure the formula for every value and stopped here: from math import floor def get_backstab_bonus_damage1(level): if level<18: return "Level up, you scrub, and learn to do it from behind" elif level>=18 and level<=38: return floor(24 + (1 + 1. / 3) * (level - 18)) elif level>=39 and level<=43: return "Someday I will calculate this, in the meanwhile just level up" elif level>=43 and level<=48: return 67 + 4 * (level - 44) elif level>=49 and level<=53: return 63 + 5 * (level - 44) elif level>=54 and level<=79: return "Why on earth would you want data for this? Karazhan? seriously" elif level>=80 and level<=85: return 310+(level-80)*7

def get_backstab_bonus_damage2(level):
    backstab_tuple = (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 24,
              25, 26, 28, 29, 30, 32, 33, 34, 36, 37, 38, 40, 41, 42, 44,
              45, 46, 48, 49, 50, 51, 53, 56, 59, 63, 67, 71, 75, 79, 83,
              88, 93, 98, 103, 108, 114, 119, 125, 131, 137, 157, 164, 170,
              176, 182, 188, 195, 201, 208, 215, 222, 229, 237, 244, 252,
              260, 268, 276, 284, 293, 301, 310, 317, 324, 331, 338, 345)
    return backstab_tuple[level]

print get_backstab_bonus_damage1(85)
print get_backstab_bonus_damage2(85)

Either way we are to expect some big chunks of code (and we have quite a hefty amount of varying numbers)

[Aldriana]

I'm of the opinion that trying to support all levels is more or less a complete waste of time. Attempting to use this framework for level 30 play has far larger issues than the lack of rating conversions - namely, the fact that the models inevitably assume certain assumptions around cycles that - while they might be valid at 80 and 85 - aren't going to be valid as 60, much less 40 or 20. Moreover, the implicit assumption in this sort of model is that you're optimizing for a fight of some reasonable duration - the averaging and whatever that we do just won't work on a fight that's only 10 or 20 seconds long.

Hence: since the answers are meaningless anyway, there's really no point in having stat information for every single level. 80 and 85? Sure. One or two others? Maybe. But I suspect it'd be far better to store the information as backstab_bonus_damage_dict = {80: 310, 85: 345} etc. rather than the full tuple.

Now, we'd need to do that for all damage formulas, rating conversions, base stats, and maybe one or two other things; and if we're going to support target level, base miss/dodge/parry rates, armor values, glancing rate, crit reduction, etc.; but we don't need to store 85 possible values for each of those We need, like, 3 or 4 per. Or at least, that's how I'd approach it.

[raconzor]

I'd just like to agree with Aldriana on that.

[dazer]

I must admit my head went trippy when thinking about this. It makes sense now. I've parsed the data I already had in my spreadsheets; leaving this here in case someone wants to start toying with it. bs_bonus_dmg_dict = {80:310, 81:317, 82:324, 83:331, 84:338, 85:345} mut_bonus_dmg_dict = {80:180, 85:201} ss_bonus_dmg_dict = {80:180, 81:184, 82:188, 83:192, 84:196, 85:200} ambush_bonus_dmg_dict = {80:330, 81:338, 82:345, 83:353, 84:360, 85:368} vw_base_dmg_dict = {80:363, 85:675} vw_percentage_dmg_dict = {80:.135, 85:.176} ip_low_base_dmg_dict = {80:300, 85:303} ip_high_base_dmg_dict = {80:400, 85:401} dp_base_dmg_dict = {80:296, 85:540} dp_percentage_dmg_dict = {80:.108, 85:.14} wp_base_dmg_dict = {80:231, 85:231} # missing lvl-85 data wp_percentage_dmg_dict = {80:.036, 85:.036} # missing lvl-85 data garrote_base_dmg_dict = {80:119, 81:122, 82:125, 83:127, 84:130, 85:133} rup_base_dmg_dict = {80:127, 81:130, 82:133, 83:136, 84:139, 85:142} rup_bonus_dmg_dict = {80:18, 81:19, 82:19, 83:19, 84:20, 85:20} evis_base_high_dmg_dict = {80:493, 81:501, 82:508, 83:516, 84:523, 85:531} evis_base_low_dmg_dict = {80:165, 81:167, 82:170, 83:172, 84:175, 85:177} evis_bonus_dmg_dict = {80:481, 81:488, 82:495, 83:503, 84:510, 85:517} env_bonus_dmg_dict = {80:216, 81:221, 82:226, 83:231, 84:236, 85:241}

[dazer]

See https://github.com/dazer/ShadowCraft-Engine/commit/b0967a3988d3b675dda4f8d6298129af667852e2 Is this what we need? By the way, most of the values are from wowhead (except those from which we have some testing: poisons and vw) but the sliders there are acting funky: you get a value (defaulted to 85), slide back to 80 and then back up to 85 and get a different one; check ambush, garrote, rup, env and specially evis. Some of them are probably due to rounding but evis has me confused (and we don't have testing done on it yet).

[Aldriana]

That's part of what we need for multilievel support. The rating conversions (in stats) and the base stats (in races) need to also be updated. And we should probably add a "supported levels" variable to calcs that is checked on init when a level is passed in. For that matter, we also need to find a decent place to pass in character level.

[raconzor]

As far as passing it in, I added this as a default parameter to the DamageCalculator class a little while ago - constructor sets self.level.

[Aldriana]

So, having screwed around with this a bit to try to figure out why the DPS numbers for level 80 are so absurdly high, I've found two things we should consider fixing if we want true multilevel support.

The first is that you basically have to enter your level twice, in two different places, to make calculations work - that is, you have to enter it both under your race to get correct base stats, and again to the Calcs object itself to get ratings, etc. It seems like it would be good if class only needed to be entered once and both usages of it automatically picked up the changes.

The second issue is that horn or winter scales with level. Most of the buffs are just percenatage boosts that disregard level, but Horn of Winter/Strength of Earth/etc. is 155 str+agi at level 80 but 549 at level 85. It would be sort of nice if there was a way for buffs to automatically pick up the level as well and return the correct value.

[dazer]

Excuse my incompetence, but I fail to see how that could be done properly: for current architecture, if level is an attribute of calcs then the only way I see to make it reach race and buffs objects would be to pass it in every level related call. I find that kind of awkward.

Would it make sense to make it a stat (in stats.py)? that way, at least, we get rid of odd calls within the modeler and the damage calculators (except for the level dependant dictionaries that is). I mean: why do we need to pass level when requesting hit_rating? if it was in stats, the get_hit_from_rating would have it already as a parameter in "self", right? I'll be pulling a request on this and see if you like it.

Making a unique call for level: if level is a stat, could we make a Character class parent to buffs, stats and the like? that way we would feed level only to the Character class (rogue would be a subclass?). This may be a rather ingenuous question, as I really don't know that much OOP to be certain of how that would look.

About the dps output: even if we set buffs/race to level 80 and choose other daggers (some 450 damage) the dps goes up to 24.5k. Barring the fact that the target is hardcoded to level 85 boss (with its higher armor) its still a pretty high figure. I'll try and break through this but... uhm... I'm running out of ideas :(

[raconzor]

Well, another alternative is to set stats.level, race.level, etc in the calculator constructor when all of these get passed in (along with level). Stats are tied to level, but race and buffs doesn't know about the stats object so that doesn't seem like the best solution. The character base class ... well, it feels a little weird to add that when all it would hold is level, but that sounds like a workable alternative.

[Aldriana]

I think having to pass level around in every function call is subotimal, and if the choice is between that and making people manually synchronize their buff level, race level, stats level, and Damage Calculator level, then maybe we have to make people manually synchronize. I just wanted to get other people thinking about the issue to see if a clever workaround can be devised.

That said: in the process of composing this post (which was originally much longer) I actually had some ideas for how to attack this, so I'm likely going to take some time this evening and/or tomorrow to see if I can come up with something.

[Aldriana]

So, I haven't 100% sorted out the details, but here's the idea I've been thinking about - let me know what you think.

The basic idea is that for level dependent objects, you define an internal function called something like _set_constants_for_level(self, level) which sets all level-dependant values to those for the relevant level. So, for instance, for buffs, you'd have something like this:

str_and_agi_buff_values = {80: 155, 85: 549}

def _set_constants_for_level(self):
    self.str_and_agi_buff_bonus = self.str_and_agi_buff_values[self.level]

def buff_str(self):
    if self.str_and_agi_buff:
        return self.str_and_agi_buff_bonus
    else:
        return 0

That is: once you call _set_constants_for_level, you no longer need to pass the level information around when accessing the data - it'll automatically give the values for that level until you again call _set_constants_for_level.

To make this easy to use, you then overwrite setattr:

def __setattr__(self, name, value):
    object.__setattr__(self, name, value)
    if name == 'level':
        self._set_constants_for_level()

Thus, when you call self.level = 85, it automatically resets all those constants to level 85 values. This could also be added to init with level passed in with a defaulted value; this would allow the object to work without needing to manually set the level, which may or may not be construed as a good thing.

Anyway, we change buffs, stats, and races to all have this behavior, and then DamageCalculator as well (i.e., when you set DamageCalculator.level it automatically sets self.buffs.level, self.stats.level, and self.race.level as well) and at this point (almost) everything automatically has the right level behavior without it needing tp be passed in anywhere.

Getting the values correctly set in RogueDamageCalculator is similar, with the added complication of making sure that both the DamageCalculator versions of the functions and the new stuff needed for RogueDamageCalculator get called. Which seems perfectly doable in theory though I admit there will probably be some picky details in practice.

Thoughts? Comments? Alternate ideas? Does this seem like a reasonable approach to people, and if so does anyone have a particular urge to implement it or should I start work on it?

[julienp]

I like the idea, something like self.evis_base_dmg_low is a lot nicer than self.EVIS_BASE_LOW_DMG[self.level] I think I can get it done this afternoon.

[Aldriana]

This seems to be reasonably well handled for the moment, so closing.