Any feedback on making custom built in functions?

Help and Feedback Code Review
#1

I cannot use a file rn (sorry)

I am not satisfied with how I cannot come up with custom function ideas and how bad they are.
I have tried looking at youtube.com videos for ideas and devforum.roblox.com
I want to improve it by making more built-in-functions and also less messy code inside my functions.

-- Custom Built-in-functions Module
local functionMod = {}

function functionMod:FindFirstDescendantOfClass(instancePart,class)
	for _, part in pairs(instancePart:GetDescendants()) do
		if part:IsA(class) then
			print("Found " .. class .. " with the name of " .. part.Name)
			break
		elseif not part:IsA(class) then
			warn(part.Name .. ":" .. " is not a " .. class)
		end
	end
end

function functionMod:BreakAttachments(instancePart)
	for _, attach in pairs(instancePart:GetDescendants()) do
		if attach:IsA("Attachment") then
			print(attach.Name .. ":" .. " Destroyed")
			attach:Destroy()
		elseif not attach:IsA("Attachment") then
			warn(attach.Name .. ":" .. " is not a Attachment")
		end
	end
end

function functionMod:PrintTable(tableName)
	for tableKey, tableVal in pairs(tableName) do
		print(tableKey .. ":", tableVal)
	end
end

function functionMod:Visibility(instancePart,amount)
	instancePart.Transparency = tonumber(amount)
end

function functionMod:MultiClone(instancePart,waitTime,cloneAmount,parent)
	local cloned = 0
	
	while wait(waitTime) do
		if cloned >= tonumber(cloneAmount) then
			break
		end
		cloned += 1
		instancePart:Clone().Parent = game:FindFirstChildOfClass(parent)
	end
end

function functionMod:ChangeColor(instancePart,color)
	instancePart.BrickColor = color
end

function functionMod:WaitForChildOfClass(instancePart,class)
	
	for _, value in pairs(instancePart:GetDescendants()) do
		
		repeat wait() until value:IsA(class)
		
		print(value.Name)
	end
end

function functionMod:PartPosition(instancePart,part)
	
	if instancePart and part then
		instancePart.Position = part.Position
	end
end

function functionMod:DestroyAllChildren(instancePart)
	
	for _, child in pairs(instancePart:GetChildren()) do
		if child then
			child:Destroy()
		end
	end
end

function functionMod:TeleportTo(instancePart,part)
	
	if instancePart and part then
		if instancePart:IsA("Model") then
			if instancePart.PrimaryPart then
				instancePart:SetPrimaryPartCFrame(part.CFrame)
			else
				warn("Model needs PrimaryPart to call this function!")
			end
		end
	end
end

function functionMod:SetGravity(gravity)
	
	game.Workspace.Gravity = tonumber(gravity)
	
end

function functionMod:FindPlayer()
	for _, player in pairs(game.Players:GetPlayers()) do
		if player then
			return player
		end
	end
end

function functionMod:Insert(id)
    local insertService = game:GetService("InsertService")

    if id then
        insertService:LoadAsset(tonumber(id))
    end
end

return functionMod
1 Like
#2

Sorry for not much detail :grinning_face_with_smiling_eyes:

#3

So some feedback

Firstly I would like to say that a lot of these things are a bit redundant, not sure that they would be that useful to have their own functions.

functionMod:PrintTable(tableName)
The print global already works pretty fine for printing tables (in output, less so in console if I remember correctly)

function functionMod:DestroyAllChildren(instancePart)
can’t you just destroy the parent?

functionMod:Visibility(instancePart,amount)
doesn’t seem that useful, since it’s one line anyways.

function functionMod:ChangeColor(instancePart,color)
same as above

function functionMod:SetGravity(gravity)
same as above

function functionMod:PartPosition(instancePart,part)
same as above

Ok now going over some of the actual functions that could be interesting to use
I think they may have some purpose, and are interesting

Firstly, you really shouldn’t use the “:” for functions unless you need the self argument

local stuff = {}
stuff.RandomNumber = 12
function stuff:Print()
    print(self.RandomNumber) -- we get access to self
end

Since these are just functions, the “.” operator will work fine

local stuff = {}
function stuff.doStuff()
    print('hi!')
end

Unless you need self, usually it’s good practice to not use “:”.

This already exists technically speaking.
You can use FindFirstChildWhichIsA and activate it recursively

local lookForPart = workspace:FindFirstChildWhichIsA("BasePart", true) -- true for recursive

This function is going to give you a lot of warnings

(For all the non-attachments and by using get Descendants)

function functionMod:WaitForChildOfClass(instancePart,class)
	
	for _, value in pairs(instancePart:GetDescendants()) do
		
		repeat wait() until value:IsA(class)
		
		print(value.Name)
	end
end

This one is probably the most interesting and I would say useful. Although you’ve implemented it incorrectly. What it does right now is loops through everything and will repeat wait() forever if the object is not of the class.

The correct implementation would probably go along something like this

function WaitForChildOfClass(instancePart, class)
    local found = nil
    repeat
        local newAdded = instancePart.ChildAdded:Wait()
        if newAdded:IsA(class) then
             found = newAdded
        end
    until found ~= nil
    return found
end

That’s at least how I might implement that

2 Likes
#4

thank you so much I am sorry for the bad functions just my ideas

#5

Theres already an Instance:ClearAllChildren() function

#6

What if you want to keep the parent but get rid of the children?

#7

As far as organization goes, try to group similar functions into modules with names that make sense. This will help you keep your code from getting too messy.

For example, if you have a bunch of functions that deal with models, you might want to make a module named “Model.” This is what it looks like in one of my projects:

image

Also use folders. It will help you a lot later if your project gets bigger.

#8

So like Model is used for models?

#9

Thats What I mean It keeps it I forgot about the clearallchildren

#10

It can be. Just name things in a way that makes sense. You have a lot of freedom when writing code.

1 Like
#11

Firstly, you really shouldn’t use the “:” for functions unless you need the self argument

Nitpick: Also you shouldn’t use self if it doesn’t reference to an object (table) created from a constructor function which inherits methods or properties from a class.

1 Like
#12

I don’t see whats remotely wrong with using “:” for functions…?

#13

You should use : when it’s needed, using it for a entirely irrelevant and unnecessary purpose like “it looks good” is bad practice.

#14

How? It doesn’t harm code at all

#15

Don’t misinterpret my words, I never literally said it harms code. It is bad practice.