I made this script for a game can you guys review it?

function GetObject(fullName)
	local segments = fullName:split(".")
	local current = game

	for _,location in pairs(segments) do
		current = current[location]
	end

	return current
end
local plr = game.Players.LocalPlayer
local UIS = game:GetService("UserInputService")
local mouse = plr:GetMouse()
local camera = game.Workspace.CurrentCamera
local holding = plr.Character:WaitForChild("Holding")
local RunService = game:GetService("RunService")
focusing = ""
UIS.InputBegan:Connect(function(key)
	if key.KeyCode == Enum.KeyCode.E and mouse.Target and mouse.Target.Parent.Name == "CanGrab"  then
	
		holding = plr.Character.Holding
		holding.Value = mouse.Target:GetFullName()
		game.ReplicatedStorage.Hold:FireServer(mouse.Target:GetFullName())
		print(holding.Value)
		
	end	

end)

RunService.RenderStepped:Connect(function()
	if holding.Value ~= "" then
		local object = GetObject(holding.Value)
		if object.Parent.Name == "CanGrab" then
		object.Massless = true
		local ray = mouse.UnitRay
		local pos = ray.Origin + (ray.Direction * 10)
		object.BodyPosition.Position = pos
		end
		end
end)

this script is used so the player can pick up some objects, how do you guys feel about it? (it also has other parts in server scripts but this is basically the main one)

1 Like

Definetely can be better in terms of performance and functionality. Is that… BodyPosition…
Is that RenderStepped instead of PreRender…?

game.Workspace instead of game:GetService(“Workspace”)

Okay. This can definetely be better in a lot of ways…

local LocalPlayer = game:GetService("Players").LocalPlayer
local Mouse = LocalPlayer:GetMouse()

local Character = LocalPlayer.Character or LocalPlayer.CharacterAdded:Wait()
local Holding = Character:WaitForChild("Holding")

local KeyCodeE = Enum.KeyCode.E
local Hold = game:GetService("ReplicatedStorage"):WaitForChild("Hold")

game:GetService("UserInputService").InputBegan:Connect(function(Input)
    local Target = Mouse.Target
    
    if Input.KeyCode == KeyCodeE and Target and Target.Parent.Name == "CanGrab" then
        Holding = Character:WaitForChild("Holding")
        Holding.Value = Target:GetFullName()
        
        Hold:FireServer(Target:GetFullName())
        
        print("Debug", Holding.Value)
    end
end)

This is the Input of how i would have done it.
I didn’ t do the last part because i couldn’ t bare the BodyPosition
BodyPosition is deprecated! Use AlignPosition or you know… Just Position or CFrame!

Also i don’ t really understand the GetObject function. Is that just GetService but uglified?

getobject is uh like the function that puts “.” between the game workspace and yuh
also thanks for helping but align position wont get me the effect i want

I can help with that. Anything for you to not use BodyPosition or Deprecated Instances. Can you tell me what you are tryng to do and why you used bodyposition for whatever you did?

i used it to make a smooth movement of the object in front of the player’s camera, its like holding something

Use the Lerp function. For the speed use tick() * 10.