Is this design good or bad? can i do something different?

Help and Feedback Code Review
, , ,
#1

i made a class for players where i can do stuff like Speed(walkspeed,seconds) where i can set the players walkspeed to some value for a certain amount of time.

i’m wondering if the method i did to achieve this could be better so that i can redesign it.

local player = {}
player.__index = player

function player.new(character)
	local newchar = {}
	setmetatable(newchar,player)
	
	player.char = character
	player.hum = character:WaitForChild("Humanoid")
	
	return newchar
end

function player:Speed(speed : number , duration : number)
	if not duration then duration = script.boost_default_time.Value end
	
	task.spawn(function()
		local cachedspeed = self.hum.WalkSpeed
		self.hum.WalkSpeed = speed
		
		task.wait(duration)
		
		self.hum.WalkSpeed = cachedspeed
	end)
end

return player

please help i want to improve

edit: i just realized the most dumbest mistake of my life, i couldve just used task.wait(seconds) instead of doing that complicated loop. i swear it nevre popped up in my head, im forgetting the most simplest things, cant believe i made a mistake like that. and yes i can use task.spawn() thats actually better thanks

#2

Only issue that really sticks out to me is your coroutine. You should just use a spawn function as using a coroutine is just like creating another script and running code from it.

spawn(function()
--Code here
end)

A coroutine is used to run code in later parts of a script but if you’re just running it after you have the coroutine, a spawn function would probably limit the amount of time it takes to execute.

Also after waitforchild make sure you add your second parameter.

:WaitForChild(ThingHere, Time) < This will basically skip over it after the amount of time listed, if it doesn’t find whatever you’re referencing instead of throwing an error and stopping the script.

Other than that, I think it’s fine.

#3

It’s a bit more complicated than it has to be - you can really just have a BaseSpeed property of the object, or even have a BaseSpeed attribute of the humanoid which will be the underlying speed which is returned to after slowdowns/speed ups. And if you implement it correctly, you could add listeners which change the speed of the humanoid accordingly to its BaseSpeed, while also taking into account of speed effects

Coroutines simply add functionality to threads, it’s not going to make a difference to the code other than clutter it if it’s not necessary. As a side note spawn is very likely to be deprecated in favor of task.spawn, along with wait for task.wait, and some others

1 Like