-
Notifications
You must be signed in to change notification settings - Fork 71
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
feature/add-auto-finance-payment-ability #60
Conversation
…oney-or-repo Feature/financing auto removes money or repo
This PR has had 60 days of inactivity & will close within 7 days |
Can this potentially be merged? |
Interresting feature ! |
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.
A few things to look at before I can recommend that this is merged. Thanks!
@@ -3,6 +3,8 @@ Config.UsingTarget = GetConvar('UseTarget', 'false') == 'true' | |||
Config.Commission = 0.10 -- Percent that goes to sales person from a full car sale 10% | |||
Config.FinanceCommission = 0.05 -- Percent that goes to sales person from a finance sale 5% | |||
Config.FinanceZone = vector3(-29.53, -1103.67, 26.42)-- Where the finance menu is located | |||
Config.FinanceAuto = true, -- Automatically take the money out of the user's account if they have it. If the time is up, they'll still need to make payment before repo... |
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.
Should be false by default
@@ -3,6 +3,8 @@ Config.UsingTarget = GetConvar('UseTarget', 'false') == 'true' | |||
Config.Commission = 0.10 -- Percent that goes to sales person from a full car sale 10% | |||
Config.FinanceCommission = 0.05 -- Percent that goes to sales person from a finance sale 5% | |||
Config.FinanceZone = vector3(-29.53, -1103.67, 26.42)-- Where the finance menu is located | |||
Config.FinanceAuto = true, -- Automatically take the money out of the user's account if they have it. If the time is up, they'll still need to make payment before repo... | |||
Config.FinanceAutoCashPayAllowed = true, -- If they don't have enough money in bank, do we take from their cash? |
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.
Should be false by default
-- Automatically take the money out from their account if they have it... If they do not have it, they have certain amount of time until it is repoed... | ||
local allPaid = true; | ||
for ind, row in pairs(result) do | ||
local paid = false; |
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.
Unnecessary var here. Can just assign allPaid = false
when a car is detected as not paid
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.
Also, you don't need semi-colons on line endings in lua
@@ -429,17 +429,62 @@ end) | |||
RegisterNetEvent('qb-vehicleshop:server:checkFinance', function() | |||
local src = source | |||
local player = QBCore.Functions.GetPlayer(src) | |||
local cash = player.PlayerData.money['cash'] |
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.
These vars should be moved down into the auto finance loop code block. Otherwise, they won't update whenever a payment is made.
player.Functions.RemoveMoney('cash', paymentNeeded); | ||
TriggerClientEvent('QBCore:Notify', src, Lang:t('general.finance_auto_paid_cash', {payment = paymentNeeded, plate = plate})); | ||
end | ||
if paid then |
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.
This if statement & the next one can be removed w/ my changes above.
local payLeft = result[ind].paymentsleft; | ||
local plate = result[ind].plate; | ||
local timer = (Config.PaymentInterval * 60) | ||
if (bank >= paymentNeeded) then |
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.
This if .. elseif
can be combined into just a single if statement.
if (bank >= paymentNeeded) or ((cash >= paymentNeeded) and (Config.FinanceAutoCashPayAllowed)) then
local paymentMethod = (bank >= paymentNeeded) and 'bank' or 'cash'
player.Functions.RemoveMoney(paymentMethod, paymentNeeded)
local newBalance, newPaymentsLeft, newPayment = calculateNewFinance(paymentNeeded, {balance = balance, paymentsLeft = payLeft})
MySQL.update('UPDATE player_vehicles SET balance = ?, paymentamount = ?, paymentsleft = ?, financetime = ? WHERE plate = ?', {newBalance, newPayment, newPaymentsLeft, timer, plate})
TriggerClientEvent('QBCore:Notify', src, Lang:t('general.finance_auto_paid_' .. paymentMethod, {payment = paymentNeeded, plate = plate}))
else
allPaid = false
end
allPaid = false; | ||
end | ||
end | ||
if not allPaid then |
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 would move this out of the if not Config.FinanceAuto then ... else
code block & re-use it rather than defining this loop twice.
end | ||
else | ||
-- Automatically take the money out from their account if they have it... If they do not have it, they have certain amount of time until it is repoed... | ||
local allPaid = true; |
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.
Move this above if not Config.FinanceAuto then ... else
and re-use it in the if not Config.FinanceAuto
section. This will help reduce code duplication (this for loop is defined twice).
TriggerClientEvent('QBCore:Notify', src, Lang:t('general.paymentduein', {time = Config.PaymentWarning})) | ||
Wait(Config.PaymentWarning * 60000) | ||
local vehicles = MySQL.query.await(query, {player.PlayerData.citizenid}) | ||
for _, v in pairs(vehicles) do |
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.
Code duplication here.
This PR has had 60 days of inactivity & will close within 7 days |
Describe Pull request
This PR adds in the ability for finances for vehicles to be automatically taken out of a user's account if the config options for it are turned on. Instead of the user needing to visit to pay the finance off, this code will make it happen automatically and update accordingly.
Questions (please complete the following information):
Yes
Yes
Yes