Input handler Module

Help and Feedback Code Review
1

The purpose of this module is to centralize all types of inputs from various sources and allow them to be used for a one purpose. For example, if I wanted to jump, the module would listen to when the space key, a UI button, or the X key on a controller is pressed. What I’m looking for is if there are areas to improve efficiency, reduce line count (I don’t care about this too much), and avoid memory leaks,

Some filler, but I haven’t programmed on Roblox for a while, and there’s something that bugs me about the code

Code:

Summary 
--!strict

local UserInputService = game:GetService("UserInputService")

local Controls = {}
Controls.__index = Controls

export type TriggerOn = "Begin" | "End"

export type ControlsConfig = {
	TriggerOn: TriggerOn?, -- "Begin" (InputBegan) or "End" (InputEnded). Default "Begin"
	IgnoreGameProcessed: boolean?, -- Default true
	Debounce: number?, 
	Enabled: boolean?, -- Start with input working?
}

export type Input = Enum.KeyCode | Enum.UserInputType | GuiButton

export type Controls = typeof(setmetatable(
	{} :: {
		Name: string,
		Inputs: { Input },
		TriggerOn: TriggerOn,
		IgnoreGameProcessed: boolean,
		Debounce: number,
		Enabled: boolean,
		_lastTrigger: number,
		_callbacks: { (InputObject?) -> () },
		_connections: { RBXScriptConnection },
	},
	Controls
	))

local function splitInputs(inputs: { Input }): ({ Enum.KeyCode | Enum.UserInputType }, { GuiButton })
	local physical: { Enum.KeyCode | Enum.UserInputType } = {}
	local buttons: { GuiButton } = {}

	for _, input in ipairs(inputs) do
		if typeof(input) == "EnumItem" then
			table.insert(physical, input :: Enum.KeyCode | Enum.UserInputType)
		elseif typeof(input) == "Instance" and input:IsA("GuiButton") then
			table.insert(buttons, input :: GuiButton)
		else
			warn(("[Controls] Unsupported input type given: %s"):format(typeof(input)))
		end
	end

	return physical, buttons
end

local function matchesInput(physicalInputs: { Enum.KeyCode | Enum.UserInputType }, inputObject: InputObject): boolean
	for _, entry in ipairs(physicalInputs) do
		if entry.EnumType == Enum.KeyCode and inputObject.KeyCode == entry then
			return true
		elseif entry.EnumType == Enum.UserInputType and inputObject.UserInputType == entry then
			return true
		end
	end
	return false
end

function Controls.new(name: string, inputs: { Input }, config: ControlsConfig?): Controls
	assert(type(name) == "string" and #name > 0, "[Controls] name must be a non-empty string")
	assert(type(inputs) == "table" and #inputs > 0, "[Controls] inputs must be a non-empty table")

	config = config or {}

	local self = setmetatable({}, Controls) :: Controls

	self.Name = name
	self.Inputs = inputs
	self.TriggerOn = config.TriggerOn or "Begin"
	self.IgnoreGameProcessed = if config.IgnoreGameProcessed == nil then true else config.IgnoreGameProcessed
	self.Debounce = config.Debounce or 0
	self.Enabled = if config.Enabled == nil then true else config.Enabled

	self._lastTrigger = 0
	self._callbacks = {}
	self._connections = {}

	self:_bind()

	return self
end

function Controls._fire(self: Controls, inputObject: InputObject?)
	if not self.Enabled then
		return
	end

	local now = os.clock()
	if now - self._lastTrigger < self.Debounce then
		return
	end
	self._lastTrigger = now

	for _, callback in ipairs(self._callbacks) do
		task.spawn(callback, inputObject)
	end
end

function Controls._bind(self: Controls)
	local physicalInputs, buttons = splitInputs(self.Inputs)

	if #physicalInputs > 0 then
		local eventName = if self.TriggerOn == "Begin" then "InputBegan" else "InputEnded"

		local connection = (UserInputService :: any)[eventName]:Connect(function(inputObject: InputObject, gameProcessed: boolean)
			if gameProcessed and self.IgnoreGameProcessed then
				return
			end
			if matchesInput(physicalInputs, inputObject) then
				self:_fire(inputObject)
			end
		end)

		table.insert(self._connections, connection)
	end

	for _, button in ipairs(buttons) do
		if self.TriggerOn == "Begin" then
			local conn = button.InputBegan:Connect(function(inputObject: InputObject)
				if inputObject.UserInputType == Enum.UserInputType.MouseButton1
					or inputObject.UserInputType == Enum.UserInputType.Touch then
					self:_fire(inputObject)
				end
			end)
			table.insert(self._connections, conn)
		else
			local conn = button.InputEnded:Connect(function(inputObject: InputObject)
				if inputObject.UserInputType == Enum.UserInputType.MouseButton1
					or inputObject.UserInputType == Enum.UserInputType.Touch then
					self:_fire(inputObject)
				end
			end)
			table.insert(self._connections, conn)
		end
	end
end

function Controls.Connect(self: Controls, callback: (InputObject?) -> ()): () -> ()
	table.insert(self._callbacks, callback)
	local callbackRef = callback
	return function()
		local index = table.find(self._callbacks, callbackRef)
		if index then
			table.remove(self._callbacks, index)
		end
	end
end

--Enable or disable input
function Controls.SetEnabled(self: Controls, enabled: boolean)
	self.Enabled = enabled
end

--Is the input on?
function Controls.IsEnabled(self: Controls): boolean
	return self.Enabled
end

-- Bypass input detection and fire event manually
function Controls.Fire(self: Controls)
	self:_fire(nil)
end

--If you're reading this RIP
function Controls.Destroy(self: Controls)
	for _, connection in ipairs(self._connections) do
		connection:Disconnect()
	end
	self._connections = {}
	self._callbacks = {}
end

return Controls
1 Like
2

that just reinventing input action system but with even more oop bloat

3

yeah agree with yarik on the abstraction part, but there’s some actual technical stuff worth pointing out too

your _bind duplicates almost the exact same connection block for buttons under Begin vs End, you already used the eventName trick for physical inputs so you could easily apply the same pattern here instead of repeating the whole thing twice

also calling task.spawn(callback, ...) on every single fire gets wasteful fast, especially for high-frequency stuff like movement keys. unless your callbacks actually yield and could block each other, just calling them directly would be a lot lighter

bigger one imo: Connect gives you a disconnect function for callbacks, but if a GuiButton gets destroyed, its connections don’t get cleaned up until you call Controls:Destroy() on the whole object. if you’re binding/unbinding short lived UI buttons a lot this’ll leak connections over time

also for debounce, os.clock() is fine but if you disable then re-enable mid debounce window, _lastTrigger just stays stale, which could cause an unexpected instant refire right after enabling

not saying scrap it though, the strict typing setup is actually pretty clean. just feels like more abstraction than the problem needs unless you’re planning on scaling this to a bunch of different control schemes later

1 Like
4

This goes for @Yarik_superpro too. The point of this is to

so basically, I add controller, keyboard, and UI triggers for an action. I really don’t want to add UserInputService every time I need to deal with input.

To reply to @BerkJR2 directly, I will take into consideration what you said.

1 Like