search

Aldriana / ShadowCraft-Engine

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

Armor mitigation #49

Closed lukfugl closed 13 years ago

lukfugl commented 13 years ago

Correct and refactor armor mitigation calculations. I'd like to take a similar approach with other level-dependent "constants", but will push just this much to get opinions first before diving into the others.

lukfugl commented 13 years ago

Pre-computing and/or caching the calculation result on the calculator object is definitely a good idea. My motivation for a separate module is just isolation of the variables and formulae relating to a specific stat for readability and testability.

julienp commented 13 years ago

Like Aldriana said, I think it's better if you use the same way we've been handling level dependent constants in other modules, see __setattr__ and _set_constants_for_level in buffs.py for an example. The way you cache it in init, if someone sets up a damage calculator with the default arguement level=85 and then changes the level to 80 or something, the calculator will still use the level 85 parameter.

Edit: that mechanism is already set up in DamageCalculator, so just cache it in _set_constants_for_level

Also, should we have .gitignore inside the git repository? I have some other things in mine, but I can work around that with the global .gitignore.

lukfugl commented 13 years ago

Regarding the .gitignore, I've always seen the project-specific .gitignore included in the project's repo for other projects (particularly work). But I'm flexible and can take it back out if we want to keep it out of the repo, I just put it in there out of habit.

Aldriana commented 13 years ago

Re: .gitignore. I'm pretty solidly indifferent on the matter. I can see value in checking it in for ease-of-setup reasons, but if people prefer to set it up themselves that's fine too.

dazer commented 13 years ago

I didn't even know .gitignore existed so I went through the files deleting *.pyc before commiting. Just like I uploaded core/files.in for tedious stuff (xgettext in my case), I believe there's some value in checking .gitignore in.

julienp commented 13 years ago

Include it then, I'm pretty much learning stuff about git as I encounter them. A quick search revealed that you can put additional per repo ignore patterns in .git/info/exclude for stuff you don't want to be shared and .gitignore is in fact meant as the way to have shared ignores.