Before the post starts, I’m new to the DevForum and this is my first post, so I’m not quite sure if I’ve formatted it 100% properly, and if I haven’t then I apologize. Please point out what I can do to make my future posts better if possible and thank you in advance.
What does the code do and what are you not satisfied with?
The code is for randomized placement of models each time a lobby is created, it does this by taking pieces from a folder in replicated storage then moving them to a position above a tile grid and relocating them to the workspace. My current issue is that because I’m new to this (this is my first time trying to develop a game), I feel like there are ways that I can cut down the script but I’m struggling with finding out how. I’ve done enough research to make it function, and I’m pleased with how it runs, but I feel like it’s inefficient. I’ve tried researching ways to improve it, but I end up in the same spot each time, probably because I don’t know what I’m supposed to be searching for.
What potential improvements have you considered?
At the current moment, the function has to check every single remaining piece of grass each time it wants to spawn a new model, I’ve considered whether or not using a table would be viable for this instead. Simply removing the selected tile from the table each time instead of having to do :GetChildren each time it spawns something in. I’ve also done research on CFrame, but given what I’m able to understand at the current moment, I’m struggling with how or if I can cut down the part where it relocates the piece.
How (specifically) do you want to improve the code?
I want to cut it down if possible to make it shorter and more efficient, it’s not like it’s bad, but if I’m going to try to learn scripting I wanted to do my best, and I feel like if I can improve it then I should do my best to find ways to improve it.
game:GetService("Workspace")
game:GetService("ReplicatedStorage")
local Island = game.Workspace.IslandMap
local InUse = Island.InUse
local AvailablePieces = game.ReplicatedStorage.Setpieces
local UseIndicator = false
-- Determines whether or not generation selection debug is enabled
Island.Grass.row4grass4.Parent = Island.InUse
Island.Grass.row4grass5.Parent = Island.InUse
Island.Grass.row4grass6.Parent = Island.InUse
Island.Grass.row5grass4.Parent = Island.InUse
Island.Grass.row5grass5.Parent = Island.InUse
Island.Grass.row5grass6.Parent = Island.InUse
Island.Grass.row6grass4.Parent = Island.InUse
Island.Grass.row6grass5.Parent = Island.InUse
Island.Grass.row6grass6.Parent = Island.InUse
-- Sets the 3x3 in the center to in-use to prevent spawns on top of them
function GrassSelector()
local GrassPieces = game.Workspace.IslandMap.Grass:GetChildren()
-- Sets the remaining grass tiles as the possible selection
local ChosenGrass = GrassPieces[math.random(1, #GrassPieces)]
-- Selects a piece of grass from the chosen row
if (UseIndicator == true)then
ChosenGrass.BrickColor = BrickColor.new("Really red")
end
if (#AvailablePieces:GetChildren() > 0) then
local Pieces = AvailablePieces:GetChildren()
local ChosenSetpiece = Pieces[math.random(1, #Pieces)]
-- Selects a random decor from the remaining setpieces
local grassX = ChosenGrass.CFrame.X
local grassY = ChosenGrass.CFrame.Y + .55
local grassZ = ChosenGrass.CFrame.Z
ChosenSetpiece:SetPrimaryPartCFrame(CFrame.new(grassX,grassY,grassZ))
-- Relocates chosen piece to select grass
ChosenSetpiece.Parent = Island.Generated
print (ChosenSetpiece.Name.." spawned on "..ChosenGrass.Name)
end
ChosenGrass.Parent = Island.InUse
end
AmountToLoop = #AvailablePieces:GetChildren()
print (AmountToLoop)
AmountLooped = 0
-- Variable used for returning data to stop the while loop
while AmountLooped ~= AmountToLoop do
GrassSelector()
AmountLooped = AmountLooped + 1
wait(.05)
end
I didn’t include the organization of my workspace because I felt like it would be useless clutter on the post, but if it’s needed I can get snips of it.
local Island = game.Workspace.IslandMap
local InUse = Island.InUse
local AvailablePieces = game.ReplicatedStorage.Setpieces
local UseIndicator = false
-- Determines whether or not generation selection debug is enabled
Island.Grass.row4grass4.Parent = Island.InUse
Island.Grass.row4grass5.Parent = Island.InUse
Island.Grass.row4grass6.Parent = Island.InUse
Island.Grass.row5grass4.Parent = Island.InUse
Island.Grass.row5grass5.Parent = Island.InUse
Island.Grass.row5grass6.Parent = Island.InUse
Island.Grass.row6grass4.Parent = Island.InUse
Island.Grass.row6grass5.Parent = Island.InUse
Island.Grass.row6grass6.Parent = Island.InUse
-- Sets the 3x3 in the center to in-use to prevent spawns on top of them
function GrassSelector()
local GrassPieces = game.Workspace.IslandMap.Grass:GetChildren()
-- Sets the remaining grass tiles as the possible selection
local ChosenGrass = GrassPieces[math.random(1, #GrassPieces)]
-- Selects a piece of grass from the chosen row
if (UseIndicator == true)then
ChosenGrass.BrickColor = BrickColor.new("Really red")
end
local Pieces = AvailablePieces:GetChildren()
if (#Pieces > 0) then
local ChosenSetpiece = Pieces[math.random(1, #Pieces)]
-- Selects a random decor from the remaining setpieces
ChosenSetpiece:SetPrimaryPartCFrame(ChosenGrass.CFrame * CFrame.new(0, 0.55, 0))
-- Relocates chosen piece to select grass
ChosenSetpiece.Parent = Island.Generated
print (ChosenSetpiece.Name.." spawned on "..ChosenGrass.Name)
end
ChosenGrass.Parent = Island.InUse
end
for i = 0, #AvailablePieces:GetChildren() do
GrassSelector()
wait(.05)
end
In my experience, there’s no need to GetService() Workspace and ReplicatedStorage. Unless you want to make a variable for ReplicatedStorage, then you would say something like local repStore = game:GetService("ReplicatedStorage") but putting the GetService() on its own line is useless. And Workspace already has a variable: workspace.
I also created the Pieces variable before checking to see if AvailablePieces was empty because calling GetChildren() twice is probably a waste of resources.
ChosenGrass.CFrame * CFrame.new(0, 0.55, 0) will set the piece to the CFrame of the grass but 0.55 studs higher. Same thing but all in one line.
The for loop at the end, again, does the same thing but cleaner. If none of this works, let me know.
It worked perfectly, I can’t thank you enough. I didn’t think I was that good at this, but 25% of the script was whopping to me. The explanations were nice and useful as well!
I had used “GetService() Workspace/ReplicatedStorage” because I thought that it would make the script halt until both those were loaded, I was afraid that the script would run before the game was loaded and it was break because the entities didn’t exist.
The Cframe cut down is big, I got tripped up trying to use Vector3/Position information and didn’t even realize it was as simple as that. Makes me feel real stupid, but then again, that’s what this post was for.
Because I never really understood For Loops when I was looking into scripting, I didn’t even think about scripting it like that. The value “i” is just a placeholder, right? and the
“0, #AvailablePieces:GetChildren” runs it for the amount counted without having to update a variable like AmountLooped?
Thanks again for helping me out, I’ll do a bit more research on everything you pointed out so I don’t pull a bozo and repeat these exact same mistakes somewhere down the line. I really appreciate you taking the time to do this!
If you want to access an object but you’re not sure if it loaded or not, you can just use :WaitForChild(). For example, game.Workspace:WaitForChild(“Part”). If you want to stop waiting after five seconds, you can do game.Workspace:WaitForChild(“Part”, 5).
for index = 0, maxNumberToLoopTo do
Usually, index or i starts at 1 but I made it 0 in this case because you made AmountLooped start at 0.
Is there no method to force a script to wait for the entire workspace to load first? Or would you just have to do something like wait(30) at the very worst? Also, thanks for the information once again!
Doing a wait(30) would be really bad because if the object that you need loads within ten seconds, you just wasted twenty seconds waiting on something that’s already in the game. It’s best to specifically wait on things that you need when you need them. If a script doesn’t need game.Workspace.Village.House.Chair, you don’t have to wait for that chair to load. But if you need game.Workspace.Village.House.Table, you can just use :WaitForChild().
To be honest, I don’t think that you need to wait for anything in Workspace unless you have StreamingEnabled. But usually, I just assume that everything is loaded in by the time I need to access something, and if I see an error in the output that says “blah blah blah is not a valid member of blah blah blah” then I just tack on a WaitForChild() and it works. Maybe that’s not a good practice, but it’s better than wait(30) lol.
I could be wrong, but I think you only need to worry about objects being loaded into Workspace if you’re trying to access those objects from the client. Everything on the server has to replicate to the client, and that’s a process. But I’m pretty sure everything in the server is already loaded from the jump, StreamingEnabled or not (since StreamingEnabled controls the replication of Workspace to the client which is irrelevant for a ServerScript accessing something on the server).
I’ll keep all this in mind, you have been a massive help to me (and to my friend making the game with me). All this information has been like a gift from the gods for idiots like us.
