How else can I improve and clean the code?

Help and Feedback Code Review
#1

How else can I improve and clean the code? The script is for character customization here is a video of what it looks like.

Code 
repeat wait() until game:IsLoaded()

local uis = game:GetService("UserInputService")

local character = game.ReplicatedStorage.Models.CharacterCustomize:Clone()
local camera = workspace.CurrentCamera

character.Parent = workspace

local animation = Instance.new("Animation")

animation.AnimationId = "http://www.roblox.com/asset/?id=507766388"
character:FindFirstChildOfClass("Humanoid"):LoadAnimation(animation):Play()

camera.CameraType = Enum.CameraType.Scriptable
camera.CFrame = CFrame.new(character.PrimaryPart.Position) * (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(0, 0, 0)) * CFrame.new(0, 0, 7)
camera.CFrame = CFrame.lookAt(camera.CFrame.Position, character.PrimaryPart.CFrame.Position)

local cameraNewLocation, characterNewLocation = camera.CFrame, character.PrimaryPart.CFrame

local player = game.Players.LocalPlayer
local gui = player.PlayerGui:WaitForChild("CharacterCustomizeGui")

local rotationFrame = gui.CharacterRotation
local MouseInDisplay, HoldInDisplay = false, false
local currentX, currentY = nil, nil

uis.InputBegan:Connect(function(input)
	if input.UserInputType == Enum.UserInputType.MouseButton1 or input.UserInputType == Enum.UserInputType.Touch then
		if MouseInDisplay == true then
			HoldInDisplay = true
			currentX, currentY = nil, nil
		end
	end
end)

uis.InputEnded:Connect(function(input)
	if input.UserInputType == Enum.UserInputType.MouseButton1 or input.UserInputType == Enum.UserInputType.Touch then
		HoldInDisplay = false
	end
end)

rotationFrame.MouseMoved:Connect(function(X, Y)
	if HoldInDisplay == false then return end

	if currentX or currentY then
		local x, y, z = camera.CFrame:ToEulerAnglesXYZ()
		
		if (Y - currentY) > 0 and x > -0.65 then
			cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(-(Y-currentY) * 0.005, 0, 0)) * CFrame.new(0, 0, 7)
		elseif (Y - currentY) < 0 and x < 0.55 then
			cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(-(Y-currentY) * 0.005, 0, 0)) * CFrame.new(0, 0, 7)
		end

		characterNewLocation *= CFrame.Angles(0, ((X - currentX) * 0.005), 0)
	end

	currentX = X
	currentY = Y
end)

rotationFrame.MouseEnter:Connect(function() MouseInDisplay = true end)
rotationFrame.MouseLeave:Connect(function() MouseInDisplay = false end)

task.spawn(function()	
	while task.wait() do
		local x, y, z = camera.CFrame:ToEulerAnglesXYZ()
		local nX, nY, nZ = cameraNewLocation:ToEulerAnglesXYZ()
		
		if x ~= nX then
			if nX < -0.65 then
				cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (CFrame.fromEulerAnglesXYZ(-0.65, 0, 0)) * CFrame.new(0, 0, 7)
			elseif nX > 0.55 then
				cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (CFrame.fromEulerAnglesXYZ(0.55, 0, 0)) * CFrame.new(0, 0, 7)
			end
			
			camera.CFrame = camera.CFrame:Lerp(cameraNewLocation, 0.2)
		end
		
		local x, y, z = character.PrimaryPart.CFrame:ToEulerAnglesXYZ()
		local nX, nY, nZ = characterNewLocation:ToEulerAnglesXYZ()
		
		if y ~= nY then
			character.PrimaryPart.CFrame = character.PrimaryPart.CFrame:Lerp(characterNewLocation, 0.1)
		end
	end
end)

3 Likes
#4

I change the MouseMoved from rotationFrame.MouseMoved:Connect(function(X, Y) to mouse.Move:Connect(function() because I also had to check if the mouse is moving for the other part of the script, and just check if the the mouse is in the frame.

And change the lerp from both character.PrimaryPart.CFrame:Lerp(characterNewLocation, 0.1) and character.PrimaryPart.CFrame:Lerp(characterNewLocation, 0.1) to TweenService

#5

Actually my bad, you’re kind of good with that, I forgot that the input object doesn’t do what it should be doing.

1 Like
#6

One improvement I can think of is some kind of user feedback via the mouse icon to indicate that you can click and drag to rotate the character.

A few code format suggestions:

  1. Assign magic numbers to constants and define them all at the top of your script. E.g. you have things like

where it is not immediately clear what the numbers -0.65, 0.55, 0.005 are doing. It also makes life easier if you want to change these values in the future.

  1. The task.spawn( while true ... end) type of loop. This would be cleaner as being an update function containing the current block of code within the loop. You can then use local connection = RunService.Heartbeat:Connect(update) to run the code when needed. This gives you the option to clean up when you no longer need the character customisation by using connection:Disconnect()
1 Like
#7

Use game:GetService() for your services.

#8

I don’t really have to use game:GetService() since, I am not changing the names of the services.

#9

It’s a good practice to use GetService, no matter if you want to give them a different name.

#10 
--------------------
-- 0952192 03.23.17
--------------------

repeat wait() until game:IsLoaded()

local uis = game:GetService("UserInputService")

local character = game.ReplicatedStorage.Models.CharacterCustomize:Clone()
local camera = workspace.CurrentCamera

character.Parent = workspace

local animation = Instance.new("Animation")

animation.AnimationId = "http://www.roblox.com/asset/?id=507766388"
character:FindFirstChildOfClass("Humanoid"):LoadAnimation(animation):Play()

camera.CameraType = Enum.CameraType.Scriptable
camera.CFrame = CFrame.new(character.PrimaryPart.Position) * CFrame.fromEulerAnglesXYZ(0, 0, 0) * CFrame.new(0, 0, 7)
camera.CFrame = CFrame.lookAt(camera.CFrame.Position, character.PrimaryPart.CFrame.Position)

local cameraNewLocation, characterNewLocation = camera.CFrame, character.PrimaryPart.CFrame

local player = game.Players.LocalPlayer
local gui = player.PlayerGui:WaitForChild("CharacterCustomizeGui")

local rotationFrame = gui.CharacterRotation
local MouseInDisplay, HoldInDisplay = false, false
local currentX, currentY = nil, nil

uis.InputBegan:Connect(function(input)
	if input.UserInputType == Enum.UserInputType.MouseButton1 or input.UserInputType == Enum.UserInputType.Touch then
		if MouseInDisplay == true then
			HoldInDisplay = true
			currentX, currentY = nil, nil
		end
	end
end)

uis.InputEnded:Connect(function(input)
	if input.UserInputType == Enum.UserInputType.MouseButton1 or input.UserInputType == Enum.UserInputType.Touch then
		HoldInDisplay = false
	end
end)

rotationFrame.MouseMoved:Connect(function(X, Y)
	if HoldInDisplay == false then return end

	if currentX or currentY then
		local x, y, z = camera.CFrame:ToEulerAnglesXYZ()
		
		if (Y - currentY) > 0 and x > -0.65 then
			cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(-(Y-currentY) * 0.005, 0, 0)) * CFrame.new(0, 0, 7)
		elseif (Y - currentY) < 0 and x < 0.55 then
			cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position) * (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(-(Y-currentY) * 0.005, 0, 0)) * CFrame.new(0, 0, 7)
		end

		characterNewLocation *= CFrame.Angles(0, ((X - currentX) * 0.005), 0)
	end

	currentX = X
	currentY = Y
end)

rotationFrame.MouseEnter:Connect(function()
	MouseInDisplay = true
end)

rotationFrame.MouseLeave:Connect(function()
	MouseInDisplay = false
end)

task.spawn(function()	
	while task.wait() do
		local x, y, z = camera.CFrame:ToEulerAnglesXYZ()
		local nX, nY, nZ = cameraNewLocation:ToEulerAnglesXYZ()
		
		if x ~= nX then
			if -0.65 > nX or nX > 0.55 then
				cameraNewLocation = CFrame.new(character.PrimaryPart.CFrame.Position)
					* (camera.CFrame.Rotation * CFrame.fromEulerAnglesXYZ(nX, 0, 0)) * CFrame.new(0, 0, 7)
			end
			
			camera.CFrame = camera.CFrame:Lerp(cameraNewLocation, 0.2)
		end
		
		local x, y, z = character.PrimaryPart.CFrame:ToEulerAnglesXYZ()
		local nX, nY, nZ = characterNewLocation:ToEulerAnglesXYZ()
		
		if y ~= nY then
			character.PrimaryPart.CFrame = character.PrimaryPart.CFrame:Lerp(characterNewLocation, 0.1)
		end
	end
end)
1 Like