Suggest Improvements for Script

Help and Feedback Scripting Support
#1

This code is some test code for a level loader (to prevent one very long loading sequence, and to improve performance).

--[[DOCUMENTATION
Program gets [x], [y], [z] coords from array and places object.
For the purpose of this script, a simple [Part] is used (game.ReplicatedStorage.Part)
]]

local xCoordinates = {10, 13, 15, 17}
local yCoordinates = {10, 11, 12, 13}
local zCoordinates = {10, 13, 15, 17}
local itemID = 1 --Not important at the moment


for i = 1, #xCoordinates do
    local xCoord = xCoordinates[i]
    local yCoord = yCoordinates[i]
    local zCoord = zCoordinates[i]

    if itemID == 1 then
        item = game.ReplicatedStorage.Part
    end

    local clonedObject = item:Clone()
    clonedObject.Position = Vector3.new(xCoord, yCoord, zCoord) --sets position
    clonedObject.Anchored = true
    clonedObject.Parent = game.Workspace["Level 0"] --Parents the object to a folder named “Level 0”
    print("Moved object to [x]: " .. xCoord)
    print("Moved object to [y]: " .. yCoord)
    print("Moved object to [z]: " .. zCoord)
end

itemID is used to differentiate between multiple models. (I used integers because that way there is less room for human error in spelling w/ strings).

How can I improve this script (performance, readability etc)?

1 Like
#2

This is fine when the coordinates all have the same number of digits, but when that’s not the case (coordinates in the 100s, 1000s, or even just negative 10s) they won’t line up vertically, so it becomes really hard to figure out which X’s, Y’s and Z’s go together.

I’d change it to this:

local points = {
    {10, 10, 10}, {13, 11, 13}, {15, 12, 15}, {17, 13, 17}
}

That way it’s not an issue even if some numbers have lots of digits, and you’ll never mess it up by e.g. removing the X coordinate but not the corresponding Y and Z coordinates, causing every coordinate to be wrong.

This is OK, but what happens when you have lots of IDs? You’ll get a long chain of if-else statements, which is a lot to write and keep track of. A more data- oriented system could automate that almost completely, e.g.:

--in a ModuleScript called "Items"
local items = {}

for _, itemModel in pairs(game.ReplicatedStorage.Items:GetChildren()) do
    local itemId = itemModel.Id.Value
    items[itemId] = itemModel
end
--Optionally add logic to check that no two Items have the same ID, and perhaps even that every ID
--    from 1 to the highest ID has an associated Item.

return items

In your script you can then do

items = require(game.ServerStorage.Items)
...
--inside the loop
local item = items[itemId]:Clone()

    print("Moved object to [x]: " .. xCoord)
    print("Moved object to [y]: " .. yCoord)
    print("Moved object to [z]: " .. zCoord)

Could be replaced with

print("Moved object to " .. clonedObject.Position)

clonedObject.Anchored = true

If you know it needs to be anchored, why not just make the original model anchored?

1 Like
#3

Thank you!
I assume that all items are children of a folder, right? (I have very little experience with scripting).