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

Provide ETA on Grimoire spell casting (Issue #136) #138

Closed
wants to merge 1 commit into from

Conversation

connorjclark
Copy link

Hovering over a spell that can't be afforded yet will display an ETA for when it can be afforded.

If a spell can be afforded, "Recovery in" relays how long until you will be back to the same mana, if you cast the spell now.

If a spell can be afforded once, but not twice, "Afford again in" relays how long until you can cast the spell again, if you were to cast it first now.

@Aktanusa Aktanusa self-requested a review August 15, 2017 22:03
@Aktanusa Aktanusa self-assigned this Aug 15, 2017
@Aktanusa Aktanusa removed their request for review August 15, 2017 22:03
@Aktanusa
Copy link
Collaborator

Aktanusa commented Sep 9, 2017

Okay, finally looking at this (sorry for the delay!) First thing I noticed is that you use the average magic per second for a second. Looking at the code, the real amount changes every single frame in a second, which means there are actually 30 different magic per second per second. Why did you opt to go this way?

@Aktanusa
Copy link
Collaborator

Aktanusa commented Sep 9, 2017

I also noticed, you removed the onmouseover backup data I saved for both buildings and upgrades. What was the reason? I guess you saw no usage of the backup data, which is true, I guess, but I wanted to backup anything I modified in the original code.

@Aktanusa
Copy link
Collaborator

Aktanusa commented Sep 9, 2017

A few more things.

My whole addon uses DOM to change things, I rather not start using string appending, like the original game does, now. (Edit: Apparently I do add none DOM stuff sometimes, my bad. I do know I tried to avoid it as much as possible though).

I already have a format time function. I don't think it was needed to remake another one.

Sorry if this is harsh criticism. I am really grateful for your help, as I still don't understand your commented math formula at all (lol), but I think I won't be using much of your code. =(. Thanks again for getting a jump start and making it easier for me to figure out how to implement it.

Aktanusa added a commit that referenced this pull request Sep 12, 2017
…pull request (Issue #138) and fixed a very minor tooltip bug
@Aktanusa
Copy link
Collaborator

Sorry, I only used this as reference. Thanks again! This feature has been added to Version 2.0042.2. I ended up not adding the "afford again" stat.

@Aktanusa Aktanusa closed this Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants