How can this code be improved?

Help and Feedback Scripting Support
#1

I’m writing a module to handle haptic inputs. I’m looking to improve the code to make it cycle though less data. I’m also looking for better practices in my code.

local hapticService = game:GetService("HapticService")

local module = {}
module.tags = {};
module.updateConnection = nil;
module.motorsToUpdate = {};

local function getClockTimeNow()
	return os.clock()
end

function module.updateHapticTag(tag : string, motorIntensities : {}, lifetime : number)

	if not motorIntensities then
		module.tags[tag] = nil
		return
	end

	for motor in motorIntensities do
		if not table.find(module.motorsToUpdate, motor) then
			table.insert(module.motorsToUpdate, motor)
		end
	end

	module.tags[tag] = {
		expiration = getClockTimeNow() + lifetime,
		motorIntensities = motorIntensities,
	};

end

module.updateConnection = game:GetService("RunService").Heartbeat:Connect(function()

	local timeNow = getClockTimeNow()
	local motorsUpdated = {}

	if not module.tags then
		return
	end

	for tag, data in module.tags do

		if not data.expiration then
			continue
		end

		if timeNow < data.expiration then

			for motor, motorIntensity in data.motorIntensities do
				motorsUpdated[motor] = (motorsUpdated[motor] or 0) + (motorIntensity or 0)
			end

		else -- tag has expired, cancel it.
			module.tags[tag] = nil
		end

	end

	for _, motor in module.motorsToUpdate do
		local intensity = motorsUpdated[motor] or 0
		hapticService:SetMotor(Enum.UserInputType.Gamepad1, motor, intensity)
	end

end)

return module
#2

this

can be shortened down to a variable

local getClockTimeNow = os.clock

other than that I’m not really noticing anything. Unless you plan on using the same module in different scripts. Since it doesn’t create a “new” module, every time it is required (from two different server script or two different local scripts on the same client) it will have the same same data and affect each other. Obviously this is not a problem if that’s intentional.

2 Likes
#3

I dont see anything really bad, but you could consider following these pieces of advice:

  • Consider employing OOP. While I’m not sure if it helps performance, it definitely make your code more readable. For example, all those variables and table you assigned to the module up there could be assigned to self instead.
  • I don’t see the need for that motorsUpdated = {}, I suppose you can use module.motorsToUpdate directly?
  • Running a loop in another loop is not performance friendly. Try to improvise your system (you don’t need to if its tedious) such that there are as few nested tables as possible to reduce number of nested loops