An Attempt to Improve Code Legibility

Hello, all.
I’m developing an SCP game by myself, and I have an obsession with making all parts of the game, be it visible or invisible to the end user perfect. This includes my code. I’m trying to make my code as legible as possible, reducing the need for refactoring and making debugging code easier.

I’m painfully aware that there are a lot of dead variables here, but it’s all done for the legibility of the code and reducing the length of lines to prevent ugly code. If this is going to cause a major problem, I’d greatly appreciate it if someone with experience could explain why.

I’ll provide screenshots below. This is because I use tabs to align my comments and some of my variables and everything would be out-of-line if I used Markdown to show you my code. :confused:

The code itself just loads some assets, increments the loading bar, displays a basic welcome message then transitions to the main menu (which is incomplete, so the functions pretty much empty). I would greatly appreciate it if any of you could suggest any ways I could make my code more legible for me or again, highlight any potential performance issues that could come with the large amount of dead variables I have and how I could possibly tackle this issue.



Thanks,
CyroStorms

2 Likes

Your effort to comment on your code is commendable, but it’s a bit excessive. There’s no need to pair every line of code with a comment—that just leads to a waste of time when the code clearly explains what’s going on.

Consider line 28 of your code: LoadAssets() -- 3. Load assets.
This comment is redundant; it adds no value to what’s already there.

I would argue that there’s no single right way to comment; there are, however, wrong ways to comment, and what you’re doing is part of the latter group. I encourage you to look up commenting practices and read a few guides.

While on the topic of formatting, I think it’s up to you to decide whether or not you like to align your variable declarations. I only do this in very specific cases (can’t think of one off the top of my head), but hey, if it helps you then sure.

I think that the long chain of dashes to separate functions is unnecessary and unhelpful. Personally, I think it’d be more helpful if you used that separator on clusters of functions instead of between every function.

While on the topic of functions, it might be useful to consider what code should be in a function and what shouldn’t. The LoadAssets function is a bit unnecessary for me. You’re only going to load assets one time, so is it really necessary to have that code be in a function (where the implication is that you’ll call it several times?). Again, what should be in a function or shouldn’t is a bit subjective, so I encourage you to read on what others have to say about that.

Some other notes:

  • What’s with the space when you call a function (between the function name and the first parenthesis)?
  • You might want to localize your functions for consistency’s sake (or for speed, but with Luau I doubt that’s an issue). There are cases when you might need to globalize a function, but I don’t see that applying here.

Also, to answer your question about dead variables (I assume you mean variables that aren’t referenced), they should not have a performance impact. (For dead local variables, they just eat at your local limit, but you shouldn’t be hitting that anyways.)

2 Likes

Yeah most language style guides actually say software developers should not use documentation patterns that are difficult to reproduce or excessive in nature. Intuitive naming conventions, casing, and appropriate spacing should be enough on top of comments explaining only lines where things are not evident.

Also, I suggest modularizing your code into single responsibility principle-compliant libraries. Basically, what I mean is that each module should be a class or library of methods which collectively have a single goal or responsibility. This way, commenting will be reduced, because all of the code in the “StatHandler” module, for example, deals with player statistics in-game and only confusing lines with weird logic or complex nested structures would require commentary.

Think about the audience of your code: you. If your team is larger, you may want to consider readability more, but if it’s just you, do what is comfortable while being reasonable with design so you can recall what an old code segment you wrote does.

2 Likes

Over some coffee, I decided to write a rendition of the snippet using a internally consistent style-guide with the subjective flavours you already decided on (double quotes, no semicolons, et cetera) If you have any questions, I heavily recommend you ask!

-- Vertical alignment is typically a bad practice in many style guides.
-- I highly recommend against it.
local StarterGui = game:GetService("StarterGui")
local ReplicatedFirst = game:GetService("ReplicatedFirst")
local ContentProvider = game:GetService("ContentProvider")

-- I recommend naming this variable to LocalPlayer; there's no need to
-- rename this variable. Not changing it however is totally fine, this
-- is only a quarrel with subjective maintainability.
local Player = game.Players.LocalPlayer
-- I recommend not naming instances that can not be indexed through the
-- dot operator. There's very little need for it.
local ModuleScripts = Player.PlayerScripts.ModuleScripts
-- Following can be PascalCase as they represent, possibly, a utility class of sorts.
local InterfaceManager = require(ModuleScripts.InterfaceManager)
local ClientConfiguration = require(ModuleScripts.ClientConfiguration)

-- I recommend using camelCase variable names for values not directly
-- corresponding to an instance in your game. It also seems like itm might be
-- better to export a table named "durations" or similar.
local transitionDuration = ClientConfiguration.transitionDuration
local greetingLogoTransitionDuration = ClientConfiguration.greetingLogoTransitionDuration
local greetingDuration = ClientConfiguration.greetingDuration

-- Although this is an array of instances, I recommend opting to camelCase since
-- the variable itself is a table.
local assets = script.Parent.Assets:GetChildren()
-- "Bar" can be more descriptive?
local Bar = script.Parent.ProgressBar.Bar
local Logo = script.Parent.Contents.Logo
local Title = script.Parent.Contents.Title
local Subtitle = script.Parent.Contents.Subtitle

-- The following is more verbose than the operation itself. While verbosity is good
-- this level of it may be unhealthy.
-- local TotalNumberOfAssets = #Assets

local loadedNumberOfAssets = 0;

local function transitionToGreeting()
	InterfaceManager.hideMenu(Bar.Parent)
	wait(transitionDuration)
	local easingDirection = Enum.EasingDirection.InOut
    -- Avoid these magical numbers; perhaps rename this variable? What does this position entail?
	local newPosition = UDim2.new(0.5, -260, 0.5, 0)
	InterfaceManager.moveElement(Logo, 1, easingDirection, newPosition)
	wait(greetingLogoTransitionDuration)
	greetPlayer()
end

local function incrementBar()
	loadedNumberOfAssets = loadedNumberOfAssets + 1
	local progression = loadedNumberOfAssets / #assets
	-- I recommend member functions are camelCase as well.
	InterfaceManager.updateBar(Bar, progression);
end

local function loadAssets()
	ContentProvider:PreloadAsync(assets, incrementBar)
	wait(transitionDuration)
	transitionToGreeting()
end

local function configureInterface()
	StarterGui:SetCore("TopbarEnabled", false)
	ReplicatedFirst:RemoveDefaultLoadingScreen()
	loadAssets()
end

-- First big derivation; introduction of helper function.
local function hourToAppropriateGreeting(hour)
    -- While I recommend avoiding magic numbers, these are not so magical as to make my head spin.
	return hour <= 5 and ClientConfiguration.EarlyMorningGreeting
		or (6 <= hour and hour <= 11) and ClientConfiguration.MorningGreeting
		or (12 <= hour and hour <= 18) and ClientConfiguration.AfternoonGreeting
		or ClientConfiguration.EveningGreeting
end

function greetPlayer()
	-- Avoid unnecessary element access expressions; I recommend property
	-- access expressions when possible as do most style guides.
	local hour = os.date("*t", os.time()).hour
	local appropriateGreeting = hourToAppropriateGreeting(hour)
	local greeting = ClientConfiguration.LoadingMenuGreeting
	-- Avoid shadowed variable
	local formattedGreeting = string.gsub(greeting, "<Greeting>", appropriateGreeting)
	formattedGreeting = string.gsub(formattedGreeting, "<Username>", Player.name)
	Subtitle.Text = formattedGreeting
	
	InterfaceManager.showMenu(Title.Parent)
	wait(greetingDuration)
	InterfaceManager.hideElement(Subtitle)
	Subtitle.Text = "Created by CryoStorms"
	InterfaceManager.showElement(Subtitle)
	wait(greetingDuration)
	transitionToMainMenu()
end
1 Like