Placement system not working

[IAmBanFor_Devforum (DevforumAccount)]

I want to create a placement system using OOP. For some reason the module script does not work.

I did not look for any help on the DevForum.

PlacementSystem:

--!nonstrict

--Grandpa mod
--Placement stuff

--Erro messages
local ERROR_MESSAGE_INVAILD_BLOCK = "Given block called %s is invaild (%s does not exist)."
local ERROR_MESSAGE_NO_ACTIVATION = "Forgot to run :Activate() thus can't place block."

--Numbers
local GRID_SIZE = 4

--Services
local UserInputService = game:GetService("UserInputService")
local RunService = game:GetService("RunService")

--Instances
local Blocks = script.Parent:WaitForChild("Blocks")
local Camera = workspace.CurrentCamera

local PlacementSystem = {}
PlacementSystem.__index = PlacementSystem


local function SnapToGrid(Number)
	--Grid snapping
	
	local x = (Number / GRID_SIZE) + 0.5
	
	return math.floor(x) * GRID_SIZE
end

local function GetMouseWorldPosition(MousePosition)
	--Gets a Vector2 mouse position and spits out a Vector3 one.
	
	local WorldPosition = Camera:ScreenPointToRay(MousePosition.X, MousePosition.Y + 36, 0)
	
	return WorldPosition
end


function PlacementSystem.new(CancelKey, GridSize)
	--OOP stuff
	
	local self = setmetatable({}, PlacementSystem)
	
	self.CancelKey = CancelKey
	self.Placing = false
	self.CurrentBlock = nil
	
	GRID_SIZE = GridSize
	
	return self
end

function PlacementSystem:Activate(BlockName)
	--Turns on placing mode
	
	local Block = Blocks:FindFirstChild(BlockName)
	
	assert(Block, ERROR_MESSAGE_INVAILD_BLOCK:format(BlockName, BlockName))
	Block = Block:Clone()
	
	self.Placing = true
	self.CurrentBlock = Block
	
	Block.Parent = workspace:WaitForChild("PlacementBlocks")
end

function PlacementSystem:Deactivate(BlockName)
	--Turns off placing mode and destroys the block
	--ONLY SHOULD BE USED WHEN CANCELING A PLACEMENT!!!!
	
	local Block = workspace:WaitForChild("PlacementBlocks"):FindFirstChild(BlockName)

	assert(Block, ERROR_MESSAGE_INVAILD_BLOCK:format(BlockName, BlockName))
	
	self.Placing = false
	self.CurrentBlock = nil
	
	Block:Destroy()
end

function PlacementSystem:Placing()
	--Handles the block moving stuff
	
	--Bad idea
	--assert(self.CurrentBlock, ERROR_MESSAGE_NO_ACTIVATION)
	--assert(self.Placing, ERROR_MESSAGE_NO_ACTIVATION)
	
	if self.CurrentBlock and self.Placing then
		local MousePosition = UserInputService:GetMouseLocation()

		MousePosition = GetMouseWorldPosition(MousePosition)
		local PosX, PosY, PosZ = SnapToGrid(MousePosition.X), MousePosition.Y, SnapToGrid(MousePosition.Z)

		self.CurrentBlock:PivotTo(CFrame.new(PosX, PosY, PosZ))
	end
end

task.spawn(function()
	RunService:BindToRenderStep("Placing", Enum.RenderPriority.Input.Value, function()
		PlacementSystem:Placing()
	end)
end)

return PlacementSystem

LocalScript:

local placement = require(game:GetService("ReplicatedStorage").PlacementSystem).new("C", 4)

wait(2)

placement:Activate("Block")

[ThanksRoBama (Emma)]

@IAmBanFor_Devforum

task.spawn(function()
	RunService:BindToRenderStep("Placing", Enum.RenderPriority.Input.Value, function()
		PlacementSystem:Placing()
	end)
end)

You don’t need to wrap this in a task.spawn since BindToRenderStepped doesn’t block the thread. The reason it doesn’t work though is because you’re effectively just calling PlacementSystem.Placing(PlacementSystem), which is not how you wrote the Placing instance method to work.

During this check in Placing:

if self.CurrentBlock and self.Placing then

… self is PlacementSystem the class, and not an instance of said class. The class doesn’t have those properties, causing them to be nil which are falsey so Placing just does nothing because you called it wrong but doesn’t throw an error because you don’t do any checks to verify the right parameters are passed.

To fix it, you need to call Placing for every instance that exists each frame. Since it appears that PlacementSystem should be a singleton, you actually don’t need to use any of the fancy stuff that’s usually associated with Lua OOP. I found it easiest to just modify your script and add comments explaining the edits:

local PlacementSystem = {}
--Since there's only one instance, these fields don't need to be stored in a table. 
--This has the added benefit that outsiders won't be able to change them, effectively making them private.
Placing = false
CurrentBlock = nil

--Don't need to set __index anymore, because we're not creating instances that need to automatically reference the class sometimes.
--Since no instances should be created we don't need a .new class method.

function PlacementSystem.Initialize(cancelKey, gridSize) --Uses . syntax instead of :, since we don't need a way of getting a reference to the instance (it's always just PlacementSystem)
	CANCEL_KEY = cancelKey
	GRID_SIZE = gridSize
	--Preventing Initialize from being called again prevents outsiders from changing constants, making "constant" a bit more meaningful
	PlacementSystem.Initialize = function() error("Cannot re-initialize singleton class 'PlacementSystem'") end
end

function PlacementSystem.Activate(blockName)
	--Turns on placing mode
	
	local block = Blocks:FindFirstChild(blockName)
	
	assert(block, ERROR_MESSAGE_INVAILD_BLOCK:format(blockName, blockName))
	block = block:Clone()
	
	Placing = true
	CurrentBlock = block
	
	block.Parent = workspace:WaitForChild("PlacementBlocks")
end

function PlacementSystem.Deactivate()
	--Turns off placing mode and destroys the block
	--ONLY SHOULD BE USED WHEN CANCELING A PLACEMENT!!!!
	
	--local block = workspace:WaitForChild("PlacementBlocks"):FindFirstChild(blockName)
	local block = CurrentBlock --Simpler and less error-prone
	assert(block, "some other error message")
	
	Placing = false
	CurrentBlock = nil
	
	block:Destroy()
end

function PlacementSystem.UpdatePlacing(dt) --A better method name	
	if not Placing then --[[If we're placing but CurrentBlock is nil, surely something is wrong?]] 
		return 
	end
	if not CurrentBlock then
		--This is a sensible way of handling CurrentBlock missing, except Deactivate throws when CurrentBlock is nil.
		--Probably just allow Deactivate to work if CurrentBlock is nil?
		PlacementSystem.Deactivate()
		return
	end

	local MousePosition = UserInputService:GetMouseLocation()
	MousePosition = GetMouseWorldPosition(MousePosition)
	local PosX, PosY, PosZ = SnapToGrid(MousePosition.X), MousePosition.Y, SnapToGrid(MousePosition.Z)
	CurrentBlock:PivotTo(CFrame.new(PosX, PosY, PosZ))
end

RunService:BindToRenderStep("Placing", Enum.RenderPriority.Input.Value, function(dt)
	PlacementSystem:UpdatePlacing(dt)
end)

return PlacementSystem