Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Squash bugs - autoRefreshPage in particular is critical. #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Squash bugs - autoRefreshPage in particular is critical. #68

wants to merge 2 commits into from

Conversation

phizch
Copy link

@phizch phizch commented Jun 22, 2015

autoRefreshPage - global variable names have changed. Would probably make browsers stop refreshing
useAutoBadgePurchase - abilityPriorityList in wrong scope
setPreference, setPreference - missing "typeof"
useAbilities - enemySpawnerHealthPercent is initialized to bool instead of number

autoRefreshPage - global variable names have changed.
useAutoBadgePurchase - abilityPriorityList in wrong scope
setPreference, setPreference - missing "typeof"
useAbilities - enemySpawnerHealthPercent is initialized to bool instead of number
@DannyDaemonic
Copy link

The only thing sitting a little off with me, and it may just be the lack of sleep, is the use of var on abilityPriorityList multiple times in the same function. Don't you only have to declare it once?

@DannyDaemonic
Copy link

The variable name corrects should go out as soon as possible. Can you drop the vars on abilityPriorityList and make it a separate pull request?

@@ -719,12 +717,11 @@ function useAutoBadgePurchase() {
//Note: this isn't an actual ratio, because badge points get reduced and the values don't add to 1
//For now, this is not a problem, but for stylistic reasons, should eventually be changed.

//Regular users buy ratio

var abilityPriorityList = []
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One var

@phizch
Copy link
Author

phizch commented Jun 22, 2015

Hi. It's the old version that has multiple vars of abilityPriorityList, not my new one.

Anyway, I'm very new to Git.. I had to delete my old repository because I had mad changes that I didn't intend to send a pull request because I didn't know of a better way...

BTW; Sorry about spamming those line comments.. I didn't know it would get shown here. Just as well maybe, since I forgot to mention them. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants