How does this tree system look?

Help and Feedback Code Review
#1

So, I wrote this for trees that players are going to be getting wood from. I was wondering if this is an okay way to go about this.

I have asked some other people and they say that using metatables is not good in this scenario, so I wanted to get the communities opinion, and wanted to see if there was a better way to do this.

The function names do exactly what they say, what do you all think?

local TreeClass = {}


TreeClass.__index = TreeClass

local StatusTypes = {"Alive", "Respawning", "Carrying", "Idle", "Moving"}

local TS = game:GetService("TweenService")

local function UpdateUI(self)
	print("Updated")
	self. Tree.Mine.AmountLeft.Wood.Amount.AmountText.Text = self.Wood .. " / " .."25"
	self. Tree.Mine.AmountLeft.Wood.Amount.Size = UDim2.fromScale(self.Wood/25,1)
end


local function CarryLog(self)
	local Info = TweenInfo.new(1)
	local Tween = TS:Create(self.Tree, Info, {CFrame = self.Tree.CFrame * CFrame.Angles(math.rad(90),0,0)})
	Tween:Play()
	self. Tree.Mine.AmountLeft.Enabled = false
	self. Tree.Anchored = false
	self.Status = StatusTypes[4]
end

local function SetupMiner(self, Backup)
	local Tree = Backup
	self.Tree = Tree
	Tree.Mine.MouseClick:Connect(function(Player)
		print(self.Status)
		if not Player then return end
		if  self.Status == StatusTypes[1] and self.Wood > 0 then
			self.Wood -= 1
			print("Ran")
			UpdateUI(self)
			self:UpdateTreeStatus()
		elseif self.Status == StatusTypes[4] or self.Status == StatusTypes[5] then
			local Char = Player.Character
			local Weld = Instance.new("Weld", Tree)
			Weld.Part0 = Tree
			Weld.Part1 = Player.Character.LeftHand
			shared.PlayerList[Player].Status = StatusTypes[3]
			self.Status = StatusTypes[3]
			shared.PlayerList[Player].CarryingItem = Tree
			--shared.PlayerList[Player]:AddItem("Wood", 1)
		end
	
	end)
end

local function MakeTree(self)
	local Tree =  self.TreeTemplate:Clone()
	local Min = game.Workspace.Forest.Min.Position
	local Max = game.Workspace.Forest.Max.Position
	local X = math.random(Min.X, Max.X)
	local Z = math.random(Min.Z, Max.Z)
	Tree.Position = Vector3.new(X,2,Z)
	Tree.Parent = game.Workspace.Forest
	SetupMiner(self, Tree)
	Tree.Anchored = true
	shared.Trees[Tree] = self
	return Tree
end

function TreeClass.GenTree(Tree, Position)
	local self = setmetatable({ }, TreeClass)
	self.TreeTemplate = Tree
	self.Tree = MakeTree(self)
	self.Wood = 25
	self.Status = StatusTypes[1]
	self.Position = Position
	return self	
end


local function Regen(self)
	wait(30)
	self.Status = StatusTypes[1]
	self.Wood = 25
	self.Tree = MakeTree(self)
end


local function RemoveStatus(self)
	coroutine.wrap(function()
		wait(30)
		if self.Status == StatusTypes[4] then
			self.Tree:Destroy()
			self.Tree = nil
		end
	end)()
end

function TreeClass:UpdateTreeStatus()
	if self.Wood <= 0 then
		CarryLog(self)
		RemoveStatus(self)
		Regen(self)
	end
end



return TreeClass
#2

It looks like code.

Could you please review the category guidelines and revise your post? Your thread is missing information, it’s just a dump of code. This isn’t a category for code appraisals as with Cool Creations, its for asking for improvements on specific parts of code that you are discontent about.

3 Likes
#3

Some examples of its use might help us.

#4

What’s the point? I don’t see how you’re gaining anything from this semi- object oriented approach. It’d be almost exactly the same if you just renamed the “self” parameter of all those functions to “tree”.

1 Like
#5

Each tree is its on “object”, when the game starts server creates a table full of trees.

Code:

local Tree = game.ServerStorage.Templates.Wood
local TreeClass = require(game.ServerStorage.Classes.TreeClass)


shared.Trees = {}

local Trees = {
	["Maple"] = 100;
}

for Num = 0,Trees.Maple do
	local TreeGen = TreeClass.GenTree(Tree)
	table.insert(shared.Trees, TreeGen)
end

What all this does:

image
image1114×311 239 KB

I use a global table to make it easier to get tree status whilst player is interacting on there plots. Each player also has a status based off of what they are doing. So when they mine a tree the player status changes to Mining.

I tried to use regular functions but I found that this is the easiest way, as coming from C++ this feels most comfortable. And the way I need it to work this feels most reliable as each tree has its own status.

Also sorry about late reply, I had work to deal with.

1 Like