Aluerie / HextechButEfficient

🔠League of Legends tool for quick & efficient management of some chores (list in README)
MIT License
8 stars 1 forks source link

Add BE Mass Disenchant everything script #11

Closed BlossomiShymae closed 9 months ago

BlossomiShymae commented 9 months ago

This PR adds the script along with integrating it with the GUI. I don't know how the script should be named exactly so it's up to you. The script was tested to work.

Please let me know if I need to make any other changes. Should resolve #10. <3

Aluerie commented 9 months ago

Sorry for the late response. I was under the weather... so i got cold... so a bit sick and lazy.


All right, If I am to merge this PR in this exact conceptuonal state then I have the following nitpicking:


but to be honest, I think the proper/more maintanable route to do this script would be to do it differently at all: rework be_mass_disenchant.py in a way we can subclass from it into be_disenchant_everything.py. Because your code is literally largely same as in the former file. So I guess, I would move the part where we decide amount of shards into new function strategy and both scripts would use same base class.

This means this PR gonna be rejected in that case, sorry :o

I can code it myself tho for this lateness as a punishment lol.

BlossomiShymae commented 9 months ago

Ah, it's okay. This meant more of you to review and get feedback since I wasn't exactly sure what to follow.

I thought about it too sharing some functionality with the previous script as well so I wasn't exactly sure what to do but I didn't want to break existing functionality. ><