SimCMinMax / AutoSimC

Python script to create multiple profiles for Simcraft to find Best-in-Slot and best enchants/gems/talents combination.
GNU General Public License v3.0
54 stars 17 forks source link

Refactor #47

Closed micolous closed 2 years ago

micolous commented 2 years ago

This is a pretty large pull request, but this is also a pretty major refactoring:

Coverage data; this does not show all files, but this is starting from 0! 😄

Name             Stmts   Miss  Cover
------------------------------------
fights.py           34      0   100%
i18n.py             48     15    69%
item.py            159     21    87%
permutation.py      69      7    90%
profile.py         250     26    90%
simc.py            111     48    57%
specdata.py         68     32    53%
utils.py            16      6    62%
------------------------------------
TOTAL              755    155    79%

TODO list:

scamille commented 2 years ago

Thanks for all the great work. Also thanks for providing such a detailed breakdown of the changes here in the pull request.

I had a quick glance over it, and it looks great so far. I want to take a deeper look at profile.py and permutation.py, since I am responsible for most of the previous rewrite of that code and am just curious of the changes there. I also want to run some simulations based on your branch to see if I run into any problems.

Can you please check which minimum python version is required for your changes? The Readme still lists 3.5, but that might need to be upgraded with all the new stuff you used, which at least I don't remember being available when working with python all those years ago.

~One thing which could also be done for the rewrite is to completely separate the nightly auto-download into a separate file while you are at it, might make main.py even smaller.~

In any case, great work and I hope we can get this merged soon. Even if there are still some minor problems, it can't be worse than the stale, nearly rotten state that AutoSimc is in right now :)

scamille commented 2 years ago

Ok I was able to take a look at it. Great work, runs fine and is much more organized.

scamille commented 2 years ago

If possible, please rebase on top of the current master. I think you might have already have incorporated my last 2 commits, if so you can just discard the incoming changes.