Random Hearthstone Toy Continued

Random Hearthstone Toy Continued

17.9k Downloads

Is there a reason the In Combat check is commented out?

teelolws opened this issue · 6 comments

commented

https://github.com/awls99/Random-Hearthstone-Toy-Continued/blob/main/RandomHearthToy.lua#L84

I suspect commenting out this line pre-dates you getting the code from the previous author? I don't know why the line is commented out, but having it commented out means the player will get an Lua error shortly after logging in if they login or /reload while in combat.

commented

Ok yeah it was Hemco. The addon was basically rewritten for v2.1: https://www.curseforge.com/wow/addons/random-hearthstone-toy/files/3154640
Compare to the previous version where the code was completely different: https://www.curseforge.com/wow/addons/random-hearthstone-toy/files/3141038

That line has been commented out since it was written, and I don't see a reason why. Should un-comment it, so that login error goes away.

commented

Yeah. I don't usually enter combat within 10s of login, but sometimes I already am in combat when I login. Following a disconnect, following a relog, etc.

commented

Good catch, thank you!
There might be something to it, so I'll run some experiments when I'm off work later.
Looking at the code I think it might have been that the author didn't want the function to do both things (check combat and update), but I'm speculating , there's a combat check in one of the places this function is used but not in the other, I suspect they thought people wouldn't get into combat within 10s of login in 🤔

commented

I couldn't get a notable difference while experimenting with or without that if commented out, here's what I ended up testing:

  1. auto attack dummies
  2. reload
  3. spam hs as soon as back from loading screen while still in combat
  4. note results
  5. stop combat
  6. spam hs button
  7. note results
  8. back in combat, attempt to HS
  9. note results

On both cases,
4 - nothing happened
7 - takes a second or so to hs to work

8 however had a difference
code as is: hs works and changes if you cancel cast.
adding the check: hs work, ICON changes, but actual hs being used stays the same

Also, I'm still not getting any lua errors in either cases, which I find odd.

Did you try it?

commented

Did you try it?

I've been uncommenting it to make the error go away for months. Have no had any problems.

Yeah it might use the wrong hearthstone to the icon, but... so what /shrug

commented

guess it might work for other people that complained about lua errors on curse forge, I'll push it.