Before you severely criticize me for having a horribly formatted script, please note than I’m a beginner and I don’t know how to organize code well.
Anyway, I made this script, but its way too long and hard to read. Is there any way I can shorten this?
local MarketplaceService = game:GetService("MarketplaceService")
local GamePassId = 24524324
MarketplaceService.ProcessReceipt = function(info)
if info.ProductId == 1218011291 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 100
else
player:WaitForChild("leaderstats").FlagBucks.Value += 200
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218011852 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 500
else
player:WaitForChild("leaderstats").FlagBucks.Value += 250
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218011973 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 1000
else
player:WaitForChild("leaderstats").FlagBucks.Value += 500
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218012019 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 2000
else
player:WaitForChild("leaderstats").FlagBucks.Value += 1000
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218012148 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 5000
else
player:WaitForChild("leaderstats").FlagBucks.Value += 2500
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218012568 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 10000
else
player:WaitForChild("leaderstats").FlagBucks.Value += 5000
end
return Enum.ProductPurchaseDecision.PurchaseGranted
elseif info.ProductId == 1218012721 then
local player = game.Players:GetPlayerByUserId(info.PlayerId)
if MarketplaceService:UserOwnsGamePassAsync(player.UserId, GamePassId) then
player:WaitForChild("leaderstats").FlagBucks.Value += 20000
else
player:WaitForChild("leaderstats").FlagBucks.Value += 10000
end
return Enum.ProductPurchaseDecision.PurchaseGranted
end
end
I’ve found a solution to your problem! You can read the comments I put under/beside the lines to get a better understanding if you’d like as you stated that you’re a beginner.
Please reply to this is you found any bugs with the below script as I’m happy to help you fix it.
--# Services.
local PlayerService = game:GetService('Players')
local MarketplaceService = game:GetService('MarketplaceService')
--# Gamepasses.
local GamepassID = 24524324
--# Product info.
local Products = {
[1218011291] = 100,
[1218011852] = 250,
[1218011973] = 500,
[1218012019] = 1000,
[1218012148] = 2500,
[1218012568] = 5000,
[1218012721] = 10000,
}
--# Receipt handling.
MarketplaceService.ProcessReceipt = function(Information)
local Leaderstats = PlayerService:GetPlayerByUserId(Information.PlayerId):WaitForChild('Leaderstats')
local AddedFlagBucks = Products[Information.ProductId] --// This searches our {Products} table for the purchaseid which returns the amount of flagbucks given.
if (MarketplaceService:UserOwnsGamePassAsync(GamepassID, Information.PlayerId)) then
AddedFlagBucks *= 2 --// I assume that this is a gamepass which gives you double the amount of flagbucks when purchased.
end
Leaderstats.FlagBucks.Value += AddedFlagBucks
return Enum.ProductPurchaseDecision.PurchaseGranted --// Be SURE to return this, as this is needed for the entire function above to complete.
end
Make sure to mark this post as a solution if it helped you!
You need more than just simplification in my opinion.
ProcessReceipt already has sample code to demonstrate how you can make the most of ProcessReceipt for multiple products if you don’t have a deep code structure for handling product purchases (e.g. module that can register ids and callbacks). It handles all products in terms of functions. This should have been your go-to reference over making your own if you’re unfamiliar with ProcessReceipt - it includes comments and you can also follow along to see how the different parts of code link to each other in order to create an easily maintainable purchase handler.
The two big problems are your choice to copy and paste the if block for one product for the rest of them as well. If you ever need to make a change it’s going to become unmaintainable for you because you would then need to apply the changes to the rest of the if blocks. Certain variables should be declared outside of your if statements so they’re accessible by the rest of the function and there’s too much repetition that needs to be cut out.
The above post provides a good frame of reference for improving your cash code however two things you might find troublesome is that it will not support non-currency products of the specific currency type (“Flag Bucks”) and it does not delay processing if an issue is encountered (player is not in the session, leaderstats doesn’t exist, UserOwnsGamePassAsync fails).
Regarding the last item in the bracketed list in the bit above, you should cache your game pass information outside of ProcessReceipt. Your game pass id belongs to a VIP game pass so other scripts may need to use it as well. If you just call UserOwnsGamePassAsync once and store a player’s VIP status you don’t have to constantly call it every time you need to apply VIP benefits. It saves you from constantly making web calls and you could apply double benefits much more easily.
Don’t think too hard about it. The explanation is in pure English so you just need to break it down into certain parts and figure out what you do and don’t know.
Most of the technical parts are in the last bit of the post so understandably someone who’s just starting out may have no idea what that means. I don’t want to make assumptions of your comprehension here but for the most part the response can be read without having any knowledge of programming.
Can you tell me what specifically confuses you from this response? Rather than rewrite my entire post, I want to grasp what you’ve taken away from this post and what’s still left that you don’t know. Take a bit of time to read it over, there’s at least one thing or two that can be understood here.
This makes sense, but I don’t know how to apply that. Are you suggesting I make a module script and store the Gamepass Ids and the other developer products so I don’t have to call then in each script I use for them?
And your part about my currency not being compatible with the script frame above if API fails for the player is confusing too.
I’m sorry if I’m bothering or annoying you. I just really want to learn more about this.
My apologies, I seen that I entered the parameters incorrectly. Replace the GamepassID in the UserOwnsGamepassAsync() with the userid first, then the gamepassid second.
The idea in the first bit is that anything that you’re expecting to access repeatedly should be held in a place that’s accessible by the rest of your code, example being the player object. ProcessReceipt is only being called for one unique purchase so cut down on the number of times you write out the player fetching by declaring the player before you start writing if statements:
function MarketplaceService.ProcessReceipt(receiptInfo)
local player = Players:GetPlayerByUserId(receiptInfo.PlayerId)
This ties into the next point which is your confusion about the cache. When I say cache I’m using it’s literal definition in that you store away something somewhere accessible to use later. It’s effectively the same reasoning as above in reducing the number of times you call something. Understanding that you are a beginner though and it’d take an ample amount of time to explain how to cache right because that’d have implications for the rest of your experience’s systems, you can just make a simple call at the top to keep their VIP status while the receipt is getting processed.
local VIP_GAME_PASS_ID = 0
function MarketplaceService.ProcessReceipt(receiptInfo)
local player = Players:GetPlayerByUserId(receiptInfo.PlayerId)
-- Don't process purchases if the player isn't in the server while the receipt
-- is being processed, otherwise they may lose money.
if not player then return Enum.ProductPurchaseDecision.NotProcessedYet end
local passGetSuccess, passGetRet = pcall(function ()
return MarketplaceService:UserOwnsGamePassAsync(receiptInfo.PlayerId, VIP_GAME_PASS_ID)
end)
if not passGetSuccess then
-- Tell Roblox to handle this purchase again later
warn("Could not fetch VIP status: " .. passGetRet)
return Enum.ProductPurchaseDecision.NotProcessedYet
end
Regarding the compatibility part, that was in reference to the code suggestion you got from post #2 which only sets up ProcessReceipt for currency purchases. If you intend to have any other developer products in your experience besides currency purchase, that script won’t be enough to get it done.
It’s incredibly important not just to think about simplifying your code but also some of the things it lacks like how it doesn’t tell Roblox to withhold the purchase if there’s an error in some part of the code. It makes lots of assumptions — even about WaitForChild returning something at all — and then implicitly grants the purchase. Monetisation needs to be treated with delicacy.