Skip to content

Conversation

@wgaylord
Copy link

@wgaylord wgaylord commented Dec 3, 2023

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.

Copy link
Owner

@ValentinFunk ValentinFunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, cool that you're implementing this! Haven't been able to run it but added some code comments

local refundsByPlayer = calculateRefundAmounts( refund, currentPrice )
-- Remove players that get refunded 0 points
local toRefund = LibK._( refundsByPlayer ):chain()
:entries( )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the filter below have amountsToRefund = entry[2]? I think it's { ownerId, amountsToRefund } so same mistake you fixed below

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, although it is working this way, so I don't think it needs to be changed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 )
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@wgaylord
Copy link
Author

Any further comments?

@ValentinFunk
Copy link
Owner

ValentinFunk commented Dec 22, 2023

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.
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