Shorten my pet buying code

Help and Feedback Code Review
#1

Provide an overview of:

  • What does the code do and what are you not satisfied with?
    my code checks if the player has enough money and gives a pet,
    im not satisfied with that i use 3 function s for 3 dispensers

  • What potential improvements have you considered?
    Make it so that it requires only 1 function to do the same

  • How (specifically) do you want to improve the code?
    i dont know

local costCommon = 100
local costVip = 250
local CostMythic = 500
function GetPolicyInfoForPlayerAsync(player)
	local p = game:GetService("PolicyService")
	local pp = p:GetPolicyInfoForPlayerAsync(player)
	return pp
end
function CommonDispenser(player)
	local PetModule = require(script.Parent.CommonPetModule)
	local Chat = game.ReplicatedStorage.moreRemoteEvents.chatWith
	if GetPolicyInfoForPlayerAsync(player).ArePaidRandomItemsRestricted then
		Chat:FireClient(player,'PolicyService says you cant do this')
		return
	end
	if not player.Character.PetInfo.petEquipt.Value then
		local buy = require(game.ServerStorage.BuyModuleScript)
		if buy.BuyWithCC(player,costCommon,'Common Egg') == Enum.ProductPurchaseDecision.PurchaseGranted then
			local pet = PetModule.Random()
			print(pet.Name..' selected')
			game.ServerStorage.playerBuysPet:Fire(player,pet.Name)
			Chat:FireClient(player,'you got a '..pet.Name)
		end
		
	else
		Chat:FireClient(player,'you cant get a pet while a pet is equipt')
	end
end

workspace.maps.City.Petshop.CommonDispenser.Text.ClickDetector.MouseClick:Connect(CommonDispenser)

function VipDispenser(player)
	local PetModule = require(script.Parent.VipPetModule)
	local mps = game:GetService("MarketplaceService")
	local Chat = game.ReplicatedStorage.moreRemoteEvents.chatWith
	if GetPolicyInfoForPlayerAsync(player).ArePaidRandomItemsRestricted then
		Chat:FireClient(player,'PolicyService says you cant do this')
		return
	end
	local id = require(game.ReplicatedStorage.IdService)
	if mps:UserOwnsGamePassAsync(player.UserId,id.gamePasses.Vipp) then
		if not player.Character.PetInfo.petEquipt.Value then
			local buy = require(game.ServerStorage.BuyModuleScript)
			if buy.BuyWithCC(player,costVip,'Vip Egg') == Enum.ProductPurchaseDecision.PurchaseGranted then
				local pet = PetModule.Random()
				print(pet.Name..' selected')
				game.ServerStorage.playerBuysPet:Fire(player,pet.Name)
				Chat:FireClient(player,'you got a '..pet.Name)
			end

		else
			Chat:FireClient(player,'you cant get a pet while a pet is equipt')
		end
	else
		Chat:FireClient(player,'you dont have vip')
	end
	
end

workspace.maps.City.Petshop.VipDispenser.Text.ClickDetector.MouseClick:Connect(VipDispenser)
function MithicDispenser(player)
	local PetModule = require(script.Parent.MithicPetModule)
	local Chat = game.ReplicatedStorage.moreRemoteEvents.chatWith
	if GetPolicyInfoForPlayerAsync(player).ArePaidRandomItemsRestricted then
		Chat:FireClient(player,'PolicyService says you cant do this')
		return
	end	
	local id = require(game.ReplicatedStorage.IdService)
	local badge = require(game.ReplicatedStorage.BadgeServiceModule)
	if badge.HasBadge(player.UserId,id.BadgeIds._100Points) then
		if not player.Character.PetInfo.petEquipt.Value then
			local buy = require(game.ServerStorage.BuyModuleScript)
			if buy.BuyWithCC(player,CostMythic,'Mythic Egg') == Enum.ProductPurchaseDecision.PurchaseGranted then
				local pet = PetModule.Random()
				print(pet.Name..' selected')
				game.ServerStorage.playerBuysPet:Fire(player,pet.Name)
				Chat:FireClient(player,'you got a '..pet.Name)
			end

		else
			Chat:FireClient(player,'you cant get a pet while a pet is equipt')
		end
	else
		Chat:FireClient(player,'you dont have the 1000 points badge')
	end

end
workspace.maps.City.Petshop.MYthicDispenser.Text.ClickDetector.MouseClick:Connect(MithicDispenser)
#2

You shouldn’t have 1 function doing multiple things, it’s called the single responsibility principle

If you see yourself repeating code in different functions (or in general) you can make that it’s own function and call it within said functions.

Or if the code is the same in different functions just there are slight differences in values you can create a function and pass arguments for said values.

It looks like your pet dispenser functions are doing roughly the same thing so you can create a function called dispensePet(pet_name,other_values) and pass the necessary arguments.

3 Likes
#3

It’s not shortening the code but for

The player might be confused as to what PolicyService is so maybe add text that says something along the lines of

'Sorry but paid random items are restricted :('
#4

You have a lot of issues with repeating code and creating complex solutions to simple problems.

For example, instead of:

local costCommon = 100
local costVip = 250
local CostMythic = 500

Do:

local costs = {
   Common = 100
   Vip = 250
   Mythic = 500
}

As this not only allows for easy access within other methods, but it allows for expansion of other rarities.

For repetitive code, make variables. For example, instead of saying game.ReplicatedStorage over and over, make a variable stating local ReplicatedStorage = game:GetService("ReplicatedStorage"), and use that.