Implement Refunds at purchase price and current price#194
Implement Refunds at purchase price and current price#194wgaylord wants to merge 3 commits intoValentinFunk:masterfrom
Conversation
ValentinFunk
left a comment
There was a problem hiding this comment.
Nice, cool that you're implementing this! Haven't been able to run it but added some code comments
lua/ps2/client/tabs/management_tab/manage_items/content/cl_pointshop2content.lua
Outdated
Show resolved
Hide resolved
| local refundsByPlayer = calculateRefundAmounts( refund, currentPrice ) | ||
| -- Remove players that get refunded 0 points | ||
| local toRefund = LibK._( refundsByPlayer ):chain() | ||
| :entries( ) |
There was a problem hiding this comment.
Should the filter below have amountsToRefund = entry[2]? I think it's { ownerId, amountsToRefund } so same mistake you fixed below
There was a problem hiding this comment.
Maybe, although it is working this way, so I don't think it needs to be changed.
There was a problem hiding this comment.
Unfortunately I don't have a gmod server to test, would be good to verify this, if entry[1] is actually the ownerId the could would try to do e.g. 123.points != 0 which should error - not sure why it doesn't since you tested this
| if refundCurrentPrice then | ||
| currentPrice = table.Inherit(itemClass.static.Price, { points = 0, premiumPoints = 0 }) | ||
| end | ||
| local refundsByPlayer = calculateRefundAmounts( refund, currentPrice ) |
There was a problem hiding this comment.
Should this be calculateRefundAmounts( results, currentPrice )? Maybe the table.Inherit(itemClass.static.Price, { points = 0, premiumPoints = 0 }) part would be better to put into calculateRefundAmounts (pass in refundCurrentPrice boolean and use the current price or purchaseData depending on it)
There was a problem hiding this comment.
The issue is the itemClass does not get passed into calculateRefundAmounts (At least from what I found using MsgAll and stuff, allitle hard to debug the tables when PrintTable does nothing.)
If you can get the itemClass in it, then that would be a good idea.
There was a problem hiding this comment.
If you're up for giving that a try I think it'd make sense - in the diff here I can only see that currentPrice is passed in to calculateRefundAmounts but it's not used. Am I missing something here it looks like refunding the current price shouldn't be able to work because the value doesn't make it all the way into the calculation 😄
There was a problem hiding this comment.
Seems I copied the wrong revision of my code when making this PR. Didn't want to push the whole file I developed this in since Dinks runs on an older version of PS2. (Due to some weird bug that also involves PAC3, not sure since I haven't seen it myself) I have pushed the info now. Going to re-do my testing again tho. Just to be sure it all works properly.
Co-authored-by: Valentin <funk.valentin@gmail.com>
|
Any further comments? |
Hey sorry for the delay here and thanks for the ping! Added some comments above |
Going to re-do my testing to make sure it all still works now that I have the fully up to date version of the functions I implemented.
This fixes the refund system for deleting items and extends it,
It allows for refunding items at the purchase price (price at time the player purchased it)
OR
At current price of the item
When refunding at current price it uses the currency based on what currency was used to purchase it.
This requires the fix to LibK to be merged to actually work.