-
Notifications
You must be signed in to change notification settings - Fork 467
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
Shopsanity Affordable Logic Update #2617
Shopsanity Affordable Logic Update #2617
Conversation
…05 depending on the selected wallet tier. Updated the tooltip to reflect.
Was this due to feedback on the current implementation? |
Yeah, apparently this was how many people were expecting it to be working, and Archez put the idea forward to make it a more even spread between the available tiers as well. Briaguya said that it was worth putting in khan because it was addressing an issue with what people were expecting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot better :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad to see the behavior updated to match user expectations!
left a few comments with ways to clear up what's going on with the code to hopefully make it easier for people to parse in the future
Pushed the other changes briaguya suggested ala comments and variable names. If you're OK with keeping the functionality mentioned in the last unresolved conversation, then it's still ready for merging. |
One more function comment.
This changes the Affordable shopsanity price logic to select from an array of "affordable prices" (one per tier) based on the tier of wallet selected. Given {10, 105, 205, 505}, each progressive tier adds one of those to the selection.
Starter -> 100% 10 rupees
Adult -> Selection between [10, 105] (50% chance)
Giant -> Selection between [10, 105, 205] (33% chance)
Tycoon -> Selection between all four (25% chance)
Also updated the tooltip for the Affordable toggle to reflect this (should be obvious this is what it's doing now, instead of confusing between this and the way I had it before).
Build Artifacts