Ways to improve/optimize my collectable coin system?

I’ve recently learnt what Object Oriented Programming is and how to use it, with this new information I’ve decided to make a coin system. Are there any improvements or optimizations that I could possibly implement into this? (To both server script and module script of course)

Server Script
-- Services

local ServerScriptService = game:GetService("ServerScriptService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")
local Debris = game:GetService("Debris")

-- Variables

local DataHandler = require(ServerScriptService.DataHandler)
local coinClass = require(ServerScriptService.CoinsHandler)
local coin_spawns = game:GetService("Workspace").CoinSpawns:GetChildren()
local COIN_COOLDOWN = 1

local players_touched_coin = {}
local coins_in_circulation = {}

-- Main

game:GetService("RunService").Heartbeat:Connect(function()
	task.wait(1)
	local randomSpawn = coin_spawns[math.random(1, #coin_spawns)]
	local coin = coinClass.new(ReplicatedStorage.Coins.Coin:Clone(), randomSpawn, 10, randomSpawn.Position + Vector3.new(0.5, 0, 0))
	coins_in_circulation[coin.Model] = true
	
	if #coins_in_circulation <= #coin_spawns then
		for i = 1, #coin_spawns do
			coin.Model.Touched:Connect(function(part)
				local character = part.Parent
				local player = game:GetService("Players"):GetPlayerFromCharacter(character)

				if not players_touched_coin[player] then
					local profile = DataHandler:Get(player)
					profile.Data.Coins += coin.Value
					game:GetService("Workspace").Pickup:Play()
					coin.Model:Destroy()
					coins_in_circulation[coin.Model] = nil
					players_touched_coin[player] = true
					task.wait(COIN_COOLDOWN)
					players_touched_coin[player] = nil
				else
					return nil
				end
			end)

			randomSpawn.DescendantAdded:Connect(function(descendant)
				if descendant:IsA("Part") and descendant.Name == "Coin" and not descendant:IsA("SpecialMesh") then
					if descendant:FindFirstChild("Coin") == nil then
						coin.Model:Destroy()
					end
				end
			end)
		end
	else
		coin.Model:Destroy()
	end
end)
Module Script
local CoinsHandler = {}

-- Services

local ReplicatedStorage = game:GetService("ReplicatedStorage")


local newCoin = {} -- object oriented programing shenanigans
newCoin.__index = newCoin

-- Public Functions

function CoinsHandler.new(model, parent, value, position)
	local self = {}
	setmetatable(self, newCoin)
	
	self.Model = model
	model.Parent = parent
	self.Value = value
	model.Position = position
	
	return self
end

return CoinsHandler

Your code is quite clean and it looks like you’re on the right path. However, I challenge you to create a Pickup/Collect/Give method that gives the coin to a specified player. Without an object being able to do things (with methods), it’s not much more than a table.

1 Like

I’ll try to explain a few things the best I can.

I would say the server script should be as simple as possible. Really the only thing it has to do is create the coin, and maybe position it. The rest could be done within the coin module.

Data shared amongst instances can be added as static variables in your module. These include coins_in_circulation and players_touched_coin. Variables declared outside any functions in a module are shared with each other as long as they’re in the same environment.

You could then attach your events in coinClass.new, calling coin:Collect(player) when Touched is dispatched.

1 Like

This seems reasonable to do, simplifying my server code rather having it long and sometimes unorganized. I would reply back when I apply my changes :slight_smile:

Yup. If done right, OOP really helps to streamline your code.

-- Services

local ServerScriptService = game:GetService("ServerScriptService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")
local Debris = game:GetService("Debris")

-- Variables

local DataHandler = require(ServerScriptService.DataHandler)
local coinClass = require(ServerScriptService.CoinsHandler)
local coin_spawns = game:GetService("Workspace").CoinSpawns:GetChildren()
local COIN_COOLDOWN = 1
local Coin = ReplicatedStorage.Coins.Coin
local players_touched_coin = {}
local coins_in_circulation = {}
local PickUp = game:GetService("Workspace").Pickup
local Players = game:GetService("Players")
-- Main

local function Eventfunction()
	task.wait(1)
	local randomSpawn = coin_spawns[math.random(1, #coin_spawns)]
	local coin = coinClass.new(Coin:Clone(), randomSpawn, 10, randomSpawn.Position + Vector3.new(0.5))
	coins_in_circulation[coin.Model] = true

	if #coins_in_circulation <= #coin_spawns then
		for i = 1, #coin_spawns do
			coin.Model.Touched:Connect(function(part)
				local character = part.Parent
				local player = Players:GetPlayerFromCharacter(character)

				if not players_touched_coin[player] then
					local profile = DataHandler:Get(player)
					profile.Data.Coins += coin.Value
					PickUp:Play()
					coin.Model:Destroy()
					coins_in_circulation[coin.Model] = nil
					players_touched_coin[player] = true
					task.wait(COIN_COOLDOWN)
					players_touched_coin[player] = nil
				else
					return nil
				end
			end)

			randomSpawn.DescendantAdded:Connect(function(descendant)
				if descendant:IsA("Part") and descendant.Name == "Coin" and not descendant:IsA("SpecialMesh") then
					if not descendant:FindFirstChild("Coin") then
						coin.Model:Destroy()
					end
				end
			end)
		end
	else
		coin.Model:Destroy()
	end
end

game:GetService("RunService").Heartbeat:Connect(Eventfunction)

Sadly, I can’t optimize your code more without knowing how your game works.
If you want more performance instead of using modules combine all scripts into one.
So your scripts can work faster.
Having 1 local and 1 server side is best for performance.