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

Dismiss Angel/Suit Exclamation #45

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

Conversation

alexarze
Copy link

@alexarze alexarze commented Apr 6, 2016

Diminished the urgency of the suit and angel exclamation by dismissing the header notification when the accordion is opened.

alexarze added 3 commits April 6, 2016 01:25
Added dismissal of angel/suit exclamation on header for accordion when
the accordion is opened
@alexarze
Copy link
Author

Just wanted to bump this small addition- has anyone taken a look at it yet?

@Slimmmo
Copy link
Owner

Slimmmo commented Apr 14, 2016

Sorry, I've looked at it and I get where you're coming from but it's going
to re-enable the dismissed notification every time it calculates as long as
you don't have the ideal suit. Which, at least in my mind, makes this
change redundant. I can't think of a way to dismiss it without a switch for
the user to turn it off completely.

On Tue, Apr 12, 2016 at 1:01 PM, jaysoncopes [email protected]
wrote:

Just wanted to bump this small addition- has anyone taken a look at it yet?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#45 (comment)

@alexarze
Copy link
Author

Sorry about the commits- I was screwing around trying to get a page up so you could test it.

It doesn't reset every time you calculate. It resets every time there's a BETTER one, aka a new alert comes up. You should try it- it doesn't reset every time you calculate, to my knowledge.

@alexarze
Copy link
Author

Actually, my bad, I just noticed that it does come back when you calculate and an angel upgrade alert is present. Does not do so for suits, though. I'll take a look in a moment.

The angel alert no longer appears after every calculation. The alert
will ONLY show if the alert is new to the game, not to the calculation.
@alexarze
Copy link
Author

Updated! You should be able to test it out at jaysoncopes.github.io

Problem: if the user typed an angel count far above the lowest angel
upgrade (say from 1 tretrigintillion to 1 duoquadragintlillion) and
then changed it back to a lower value, the angel upgrades above the new
angel count were not dismissed as “exclamation” upgrades. This fix
removes the alert from any that are above the angel count.
@alexarze
Copy link
Author

Sorry to be seemingly spamming this pull request, but I also fixed an issue with angel upgrade recommendations (that should probably be applied to cash upgrades as well). See commit af62f60 (most recent)

@alexarze
Copy link
Author

Bump - have you had a chance to take a look at this pull yet?

@alexarze
Copy link
Author

@Slimmmo is there anything wrong with this pull request? I'd be happy to fix anything that you find less than desirable.

@Slimmmo
Copy link
Owner

Slimmmo commented Apr 26, 2016

Sorry if this is blunt but I'm not convinced it adds anything to the site. I can see why it could be useful but with the way it's implemented now it seems a little funky to me.

Let's say you input your numbers and hit calc and an angel upgrade for venture X would give you an increase in income of 0.001% but costs 5% of your angels. Not worth buying but the alert comes up since it's a slight increase in income. You look at it, decide it's not worth buying and then close the accordion to hide the alert. You then hit a 99999999x unlock for venture X which makes it your top earner and changes the angel upgrade to being worth buying but since you've already dismissed the alert and there's a non-boolean value in the array there will be no new alert and so no way of knowing a big upgrade is there unless you just remember.

Conversely, the way it's implemented at the moment could cause people to not check it since the alert is there so often.

I honestly don't know what the best solution for this is. If you change it to showing an exclamation when the percentage increases then just going +1 to a venture will cause the alert to pop back up again.

Sigh.... thoughts?

@alexarze
Copy link
Author

Actually, the way it should work (based on your code) is if the increase of profits from the angel upgrade vs. the profits from having the angels is greater, then it creates an exclamation. Your code generates an alternate world with the same stats, but with a reduced angel amount to match the cost of the upgrade and the upgrade installed. If the $/s is greater in the "alternate reality", then it gets an exclamation. Thus, you really shouldn't be seeing the exclamation unless it's "worth it" already.

I do see your argument, however: there's no great way to make it intuitive. The only thing I can say is that if the user has dismissed the notification, then we can presume they have seen the upgrade available, and would be smart enough to know that it's available. The plus sign doesn't disappear on the upgrade itself, by the way, just the accordion.

One thing that I just noticed that I don't mind fixing unless you think it's actually better is that upon refreshing, the notification appears again. If everyone uses your calculator the way I do, they probably revisit the site every once in a while to check what the next best upgrade would be, meaning that they have the opportunity to check the angel upgrades again. I might make a more permanent dismissal for the suits, though, because if someone doesn't buy them now, they probably won't later, either.

@Slimmmo
Copy link
Owner

Slimmmo commented Apr 26, 2016

There is an indicator if it is an increase in income, not a decrease in reset time. This is an important distinction as the former doesn't necessarily mean the latter when sacrificing angels.

If you make that change as well as making the indicator appear again after changing the number of angels you have then I think I'll be happy to merge it.

@alexarze
Copy link
Author

That's true. However, in the current system, that's not yet an option either, so I think that's something we can watch later.

Alright. I'll let you know when it's done.

Added notification state to JSON and importing as well as to the
liverich world
Allows for the notification to be recalculated upon changing of angels.
Also adds the suit and angel notification variable to planet
requirements.
Notification status is no longer overriden by calculations on load
Fixed issue with Suits that caused the notification not to reappear
when a better suit was available

Fixed issue where updating your angel count removed all the angel
upgrades

Updating angel count now recalculates all instead of just the angels
(forces push to local storage so as to keep changes on refresh)
@alexarze
Copy link
Author

@Slimmmo Alright, this should be ready. You can check it out at jaysoncopes.github.io. It shouldn't have a problem loading old saves, and new saves are automatically created with the new keys. Upon changing angel count, it now recalculates (technically everything, for saving purposes) and will reset the notification as you requested. As mentioned briefly above, the hasSeenAngel/SuitExclamation and angel/suitExclamation variables are loaded with the save now, allowing them to be more specific to the user and not the session. This also prevents them from being reset on reload.

Let me know if you see anything that needs changing.

@alexarze
Copy link
Author

@Slimmmo bump?

@alexarze
Copy link
Author

alexarze commented May 8, 2016

If you don't want this, please just let me know- I can close it with no hurt feelings. It was a fun project to work on, but it's agonizing not knowing what's making this dissatisfactory.

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