Include a standalone, bare bones .rbxl file with only the code you want reviewed.
Link to file download: CodeReview.rbxl (130.0 KB)
Provide an overview of:
What does the code do and what are you not satisfied with?
~| I have a GUI that is opened with the top bar icon, more specifically a lighting preset GUI. My code works in changing the players’ lighting when pressing a text button, but it is VERY repetitive and almost 200 lines of code…
~| I am wanting to find solutions on shortening or removing repeating/redundant lines of code.
What potential improvements have you considered?
~| I originally was going to do it line by line, but I opted to use folders full of different value types. The code is to loop through the desired lighting preset in the "LightingPresets" folder in ReplicatedStorage and manually set each lighting property line of code by line of code.
Structure of the lighting preset folder
How (specifically) do you want to improve the code?
~| I just want to know if there’s anything I can do to not be so repetitive and/or if there’s anything I should know about. The code is fully functional and does not need debugging. If you encounter an error, please let me know if you make any changes.
Sorry, I was just following the template the devforum provided.
local rs = game:GetService("ReplicatedStorage")
local players = game:GetService("Players")
local sg = game:GetService("StarterGui")
local l = game:GetService("Lighting")
local RunService = game:GetService("RunService")
---<GUI Structure>---
local lpGUI = script.Parent
local BGFrame = lpGUI.BackgroundFrame
local scrollCon = BGFrame.ScrollingContainer
local VibePinkPresetSubmit = scrollCon.VibePinkPresetFrame.Submit
local VibePinkPresetDesc = scrollCon.VibePinkPresetFrame.PresetDesc
local plr = players.LocalPlayer
local LightingPresets = rs.LightingPresets
local function setDefaultStudioLighting()
local defaultStudio = LightingPresets.DefaultStudio
local lp = defaultStudio.LightingProperties
local at = defaultStudio.Atmosphere
local s = defaultStudio.Sky
local dof = defaultStudio.DepthOfField
local sr = defaultStudio.SunRays
local b = defaultStudio.Bloom
if plr and plr.Character then
l.Ambient = lp.Ambient.Value
l.ColorShift_Top = lp.ColorShift_Top.Value
l.ColorShift_Bottom = lp.ColorShift_Bottom.Value
l.Brightness = lp.Brightness.Value
l.EnvironmentSpecularScale = lp.EnvironmentSpecularScale.Value
l.ShadowSoftness = lp.ShadowSoftness.Value
l.OutdoorAmbient = lp.OutdoorAmbient.Value
l.EnvironmentDiffuseScale = lp.EnvironmentDiffuseScale.Value
l.GlobalShadows = lp.GlobalShadows.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if not l.Atmosphere then Instance.new("Atmosphere", l) end
l.Atmosphere.Decay = at.Decay.Value
l.Atmosphere.Glare = at.Glare.Value
l.Atmosphere.Haze = at.Haze.Value
l.Atmosphere.Color = at.Color.Value
l.Atmosphere.Offset = at.Offset.Value
l.Atmosphere.Density = at.Density.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if not l.Sky then Instance.new("Sky", l) end
l.Sky.CelestialBodiesShown = s.CelestialBodiesShown.Value
l.Sky.MoonAngularSize = s.MoonAngularSize.Value
l.Sky.SunTextureId = s.SunTextureId.Value
l.Sky.SunAngularSize = s.SunAngularSize.Value
l.Sky.MoonTextureId = s.MoonTextureId.Value
l.Sky.StarCount = s.StarCount.Value
l.Sky.SkyboxBk = s.SkyboxBk.Value
l.Sky.SkyboxDn = s.SkyboxDn.Value
l.Sky.SkyboxFt = s.SkyboxFt.Value
l.Sky.SkyboxLf = s.SkyboxLf.Value
l.Sky.SkyboxRt = s.SkyboxRt.Value
l.Sky.SkyboxUp = s.SkyboxUp.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if not l.Bloom then Instance.new("BloomEffect", l) end
l.Bloom.Intensity = b.Intensity.Value
l.Bloom.Size = b.Size.Value
l.Bloom.Threshold = b.Threshold.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if not l.DepthOfField then Instance.new("DepthOfFieldEffect", l) end
l.DepthOfField.FarIntensity = dof.FarIntensity.Value
l.DepthOfField.FocusDistance = dof.FocusDistance.Value
l.DepthOfField.InFocusRadius = dof.InFocusRadius.Value
l.DepthOfField.NearIntensity = dof.NearIntensity.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if not l.SunRays then Instance.new("SunRaysEffect", l) end
l.SunRays.Intensity = sr.Intensity.Value
l.SunRays.Spread = sr.Spread.Value
--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--<>--
if l.ColorCorrection then l.ColorCorrection:Destroy() end
end
end
local function test(presetName)
local defaultStudio = LightingPresets.DefaultStudio
local lp = defaultStudio.LightingProperties
local at = defaultStudio.Atmosphere
local s = defaultStudio.Sky
local dof = defaultStudio.DepthOfField
local sr = defaultStudio.SunRays
local b = defaultStudio.Bloom
if not l.SunRays then Instance.new("SunRaysEffect", l) end
if not l.Atmosphere then Instance.new("Atmosphere", l) end
if not l.DepthOfField then Instance.new("DepthOfFieldEffect", l) end
if not l.Bloom then Instance.new("BloomEffect", l) end
if not l.Sky then Instance.new("Sky", l) end
if plr and plr.Character then
for i, a in pairs(defaultStudio:GetChildren()) do
if a:IsA("Folder") then
if a.Name == "LightingProperties" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "Ambient" then
l.Ambient = a2.Value
elseif a2.Name == "ColorShift_Bottom" then
l.ColorShift_Bottom = a2.Value
elseif a2.Name == "Brightness" then
l.Brightness = a2.Value
elseif a2.Name == "EnvironmentSpecularScale" then
l.EnvironmentSpecularScale = a2.Value
elseif a2.Name == "EnvironmentDiffuseScale" then
l.EnvironmentDiffuseScale = a2.Value
elseif a2.Name == "ShadowSoftness" then
l.ShadowSoftness = a2.Value
elseif a2.Name == "OutdoorAmbient" then
l.OutdoorAmbient = a2.Value
elseif a2.Name == "ColorShift_Top" then
l.ColorShift_Top = a2.Value
elseif a2.Name == "GlobalShadows" then
l.GlobalShadows = a2.Value
end
end
elseif a.Name == "SunRays" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "Intensity" then
l.SunRays.Intensity = a2.Value
elseif a2.Name == "Spread" then
l.SunRays.Spread = a2.Value
end
end
elseif a.Name == "DepthOfField" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "FarIntensity" then
l.DepthOfField.FarIntensity = a2.Value
elseif a2.Name == "FocusDistance" then
l.DepthOfField.FocusDistance = a2.Value
elseif a2.Name == "InFocusRadius" then
l.DepthOfField.InFocusRadius = a2.Value
elseif a2.Name == "NearIntensity" then
l.DepthOfField.NearIntensity = a2.Value
end
end
elseif a.Name == "Sky" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "CelestialBodiesShown" then
l.Sky.CelestialBodiesShown = a2.Value
elseif a2.Name == "MoonAngularSize" then
l.Sky.MoonAngularSize = a2.Value
elseif a2.Name == "SunTextureId" then
l.Sky.SunTextureId = a2.Value
elseif a2.Name == "SunAngularSize" then
l.Sky.SunAngularSize = a2.Value
elseif a2.Name == "StarCount" then
l.Sky.StarCount = a2.Value
elseif a2.Name == "MoonTextureId" then
l.Sky.MoonTextureId = a2.Value
elseif a2.Name == "SkyboxBk" then
l.Sky.SkyboxBk = a2.Value
elseif a2.Name == "SkyboxDn" then
l.Sky.SkyboxDn = a2.Value
elseif a2.Name == "SkyboxFt" then
l.Sky.SkyboxFt = a2.Value
elseif a2.Name == "SkyboxLf" then
l.Sky.SkyboxLf = a2.Value
elseif a2.Name == "SkyboxRt" then
l.Sky.SkyboxRt = a2.Value
elseif a2.Name == "SkyboxUp" then
l.Sky.SkyboxUp = a2.Value
end
end
elseif a.Name == "Atmosphere" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "Decay" then
l.Atmosphere.Decay = a2.Value
elseif a2.Name =="Glare" then
l.Atmosphere.Glare = a2.Value
elseif a2.Name =="Haze" then
l.Atmosphere.Haze = a2.Value
elseif a2.Name =="Color" then
l.Atmosphere.Color = a2.Value
elseif a2.Name =="Offset" then
l.Atmosphere.Offset = a2.Value
elseif a2.Name =="Density" then
l.Atmosphere.Density = a2.Value
end
end
elseif a.Name == "Bloom" then
for i,a2 in pairs(a:GetChildren()) do
if a2.Name == "Intensity" then
l.Bloom.Intensity = a2.Value
elseif a2.Name == "Size" then
l.Bloom.Size = a2.Value
elseif a2.Name == "Threshold" then
l.Bloom.Threshold = a2.Value
end
end
end
end
end
end
if game.Lighting.ColorCorrection then game.Lighting.ColorCorrection:Destroy() end
end
VibePinkPresetSubmit.MouseButton1Click:Connect(test)
-Variables are not consistant nor descriptive. you have both ‘rs’, ‘players’, and ‘RunService’ for ReplicatedStorage, Players, and RunService. I recommend PascalCase. camelCase works, but they should all use the same case and descriptive. Probably would never use the character ‘l’ as a variable
-if plr and plr.Character then I would make it if plr==nil or plr.Character==nil then return end. This will save you a scope and exit the function early.
-if not Lighting.Atmosphere That’s an error(which I ran into). use findFirstChild to handle a missing instance
-Rather than manually setting each property for lighting/bloomeffect/etc, simply make sure your LightingPresets have exact classnames and exact properties, then loop through them to create the needed instances and set the appropriate properties. This will cut your lines in half.
-Consider making one function that handles all changing of Lighting/Lighting instances, and feed it new lighting/default lighting settings accordingly. So you dont need both “set default” and “set preset”
Hi, I think the best thing for you to do is something like this:
local change = function(lightingType)
l.Atmosphere = lightType.Atmosphere or 1
l.Brightness = lightType.Brightness or 1
l.Bloom = lightType.Bloom or 1
…and so on
- also the “or 1” stuff is the default value if not provided in the lightingType table
end
change(LightTypesModule.DarkType)
Thank you for the feedback! I definitely agree on the variable issue hahaha, genuinely got a bit lazy! But I appreciate the tips and will be applying them accordingly!
