Adds creditable hints #334
Conversation
There was a problem hiding this comment.
Thanks for making these changes! I'm only asking about a few things that would consolidate much of the similar code in one file. Push back if you disagree.
Also I'd like to see @morganbr 's opinion on how this UX feels since hints are his thing. [Personally, I'm fine with the changes and I like the discount enhancement, though realistically the changes are separable if there's a reason to separate them.]
| @@ -47,19 +47,24 @@ public async Task<IActionResult> OnGetAsync(int puzzleId, int teamId) | |||
| return Page(); | |||
| } | |||
|
|
|||
| private async Task PopulateUI(int puzzleId, int teamId) | |||
| private async Task<IList<HintWithState>> GetAllHints(int puzzleId, int teamId) | |||
There was a problem hiding this comment.
This function, or another one next to it, should compute the discount so we only need to write and maintain it once and in one file.
There was a problem hiding this comment.
Similarly, how about we compute the original/adjusted hint cost in HintWithState as well.
There was a problem hiding this comment.
Both suggestions sound reasonable. There's a small problem though. I can't compute the base and adjusted costs in the HWS constructor because it requires the discount value which I can't compute until I have all the hints loaded. So I can create the hws-list as it does now, then compute the discount, then iterate over the list and call hws.SetDiscount() on each of them. Don't know what your coding conventions are here and if that breaks them.
Let me know and I'll make the change.
There was a problem hiding this comment.
Personally, I'm totally fine with that approach, though Morgan has schooled me on coding style on more than one occasion and I'd like his take. :-)
There was a problem hiding this comment.
Yes, creating the list and then modifying it sounds fine. You could add AdjustedCost as a property of HintWithState. It would help avoid having too much logic in the display layer.
There was a problem hiding this comment.
I've refactored the code as suggested. HintWithState now has both BaseCost and AdjsutedCost properties which are used instead of hws.Hint.Cost.
|
Thanks, Bruce! |
A hint with negative cost is credited towards the cost of other hints. Note that we compute the discount for other hints as the largest negative hint they've unlocked (Math.Min that is). That's because if they've unlocked two creditable hints, than one of those was credited with the cost of the other one and the net amount they paid was the largest one.
Also reduces the number of columns in the hints page to make the layout simpler. I'll send out a screenshot.