search

AlexFolland / AutoGear

WoW convenience addon that automatically handles gear looting and equipping
https://curseforge.com/wow/addons/autogear
Other
8 stars 11 forks source link

Small Cata fixes #62

Closed TyThompson closed 3 months ago

TyThompson commented 4 months ago

It isn't perfect but it is a step in the right direction

Last bug I'm looking at is AutoGear/AutoGear.lua:2101: attempt to index local 'realSpec' (a number value) [string "@AutoGear/AutoGear.lua"]:2101: in function <AutoGear/AutoGear.lua:2079>

AlexFolland commented 4 months ago

Also, I'm wondering why it's necessary to introduce an anonymous function that just calls AutoGearMain instead of just passing AutoGearMain to AutoGearFrame:SetScript directly like we've always done before this. What changed or what did you observe to make that necessary?

I realize the difference is literally just an additional function call on every update and this almost doesn't matter, but I am curious.

TyThompson commented 4 months ago

It looked strange to me so I made it a one liner like the lines above it. The old code works as well.

AlexFolland commented 4 months ago

It looked strange to me so I made it a one liner like the lines above it. The old code works as well.

OK, please change that part back then. We don't need to change that part.

Edit: Oh, I see you've already done this. Thanks.

AlexFolland commented 4 months ago

Why was this closed? Who is @wfwfewe? I've just now changed the settings to prevent non-collaborators from reviewing. I'll be checking this out this weekend.

TyThompson commented 3 months ago

Sorry first pull request I was confused after it was approved lol

AlexFolland commented 3 months ago

Oh, that makes sense! Haha, yeah, I was confused when a non-contributor with no contributions anywhere on the site was allowed to submit a code review on this project. I became aware of a setting that prevents that as a result.

Sorry, the reality of family life has meant that I haven't had any time this weekend to boot up WoW and play with this change. I plan to do it soon still, but having 2 kids makes free time very limited. I can come on here and comment once in a while when I get a couple minutes to sit down, but dedicating actual time is just not realistic sometimes.

TyThompson commented 3 months ago

Made the requested changes

AlexFolland commented 3 months ago

I'm merging this and fixing the last issue myself. Thank you very much for your effort!