Skip to content

Fix connection leaks, naming conventions, and missing Start() function #2

@Zyn-ic

Description

@Zyn-ic

Overview

While integrating Stoway my our project, I've identified several issues that should be addressed in the upstream repo. These fall into two categories: RBXScriptConnection memory leaks and naming convention inconsistencies.

1. Memory Leaks: Untracked Connections

SlotFactory.luau - Button connections not cleaned up by Fusion scope

File: src/client/StowayClientv1_2/SlotCreation/SlotFactory.luau
The original code connects MouseButton1Down, MouseButton1Click, and MouseEnter directly on the button instance, but these connections are never added to the Fusion scope. When a slot is destroyed (e.g., via ForPairs cleanup), the connections remain alive, creating a memory leak per slot.

-- ORIGINAL (leaks)
slotbutton.MouseButton1Down:Connect(function()
    if scope.peek(hasItem) and props.OnDrag then
        templateSlot.ZIndex = 1
        props.OnDrag(props.SlotType, props.SlotIndex, templateSlot, InventoryStore:GetState())
    end
end)
slotbutton.MouseButton1Click:Connect(function()
    InventoryController.HandleEquipped(scope.peek(slotUUID), true)
    if props.hookManager then
        props.hookManager:Fire("OnSlotClick", props.SlotType, props.SlotIndex, scope.peek(slotUUID))
    end
end)
slotbutton.MouseEnter:Connect(function()
    if props.hookManager and not IsActiveHover then
        IsActiveHover = true
        props.hookManager:Fire("OnSlotHover", slotbutton, props.SlotType, props.SlotIndex)
        task.wait(0.25)
        IsActiveHover = false
    end
end)

Fix: Add connections to the Fusion scope using table.insert(scope, connection):

table.insert(scope, slotbutton.MouseButton1Down:Connect(function()
    if scope.peek(hasItem) and props.OnDrag then
        templateSlot.ZIndex = 1
        props.OnDrag(props.SlotType, props.SlotIndex, templateSlot, InventoryStore:GetState())
    end
end))

Fusion scopes call Disconnect() on all entries during cleanup, which properly releases these connections.

BuildStorage (in init.luau) - Search connection not tracked

File: src/client/StowayClientv1_2/init.luau
The SearchFilter:GetPropertyChangedSignal("Text") connection is not added to the Fusion scope:

-- ORIGINAL (leaks)
SearchFilter:GetPropertyChangedSignal("Text"):Connect(function()
    SearchText:set(SearchFilter.Text:lower())
end)

Fix:

local searchConn = SearchFilter:GetPropertyChangedSignal("Text"):Connect(function()
    SearchText:set(SearchFilter.Text:lower())
end)
table.insert(StorageScope, searchConn) -- Fusion will call :Disconnect() on cleanup

ConsoleDropUI.luau - Button connections not tracked

File: src/client/StowayClientv1_2/Input/ConsoleDropUI.luau
The Drop, Dropall, Cancel, Increase, and Decrease button connections are never disconnected:

-- ORIGINAL (leaks)
Drop.Activated:Connect(function() ... end)
Dropall.Activated:Connect(function() ... end)
Cancel.Activated:Connect(function() ... end)
Increase.Activated:Connect(function() ... end)
Decrease.Activated:Connect(function() ... end)

Fix: Store connections in a table and disconnect them in ConsoleDropUI.Destroy():

local connections = {}
-- In Create()
table.insert(connections, Drop.Activated:Connect(function() ... end))
-- etc.
-- In Destroy()
for _, conn in ipairs(connections) do
    conn:Disconnect()
end
table.clear(connections)

2. Naming Convention Inconsistencies

Parameter casing: REPLICATE vs replicate

File: src/server/StowayServerV1_2/init.luau
All public API functions use UPPER_CASE parameters like REPLICATE (e.g., AddItem(player, itemId, amount, metadata, REPLICATE)). This deviates from the standard Luau convention of camelCase for parameters:

-- Original
function InventoryService.AddItem(player: Player, itemId: string, amount: number, metadata: any?, REPLICATE: boolean)
-- Suggested
function InventoryService.AddItem(player: Player, itemId: string, amount: number, metadata: any?, replicate: boolean)

This applies to AddItem, RemoveItem, SwapSlots, EquipSlot, UnequipSlot, DropItem, UpdateMetadata, StackTwoSlots, and AddToBackpack.

Method name typo: GetUUidFromIndex / GetUUidFromVisibleHotbarIndex

File: src/client/StowayClientv1_2/CoreState/InventoryStore.luau
These methods use inconsistent casing in the abbreviation "UUID":

  • GetUUidFromIndex - mixed case
  • GetUUidFromVisibleHotbarIndex - mixed case
    Fix: Rename to:
  • GetUUIDFromIndex
  • GetUUIDFromVisibleHotbarIndex
    Update all call sites accordingly (e.g., in init.luau and InventoryController.luau).

3. Missing Start() Function (Client)

File: src/client/StowayClientv1_2/init.luau
The client module exports Init() but no Start() function. If the ModuleLoader (or any loader) calls module.Start(), this will error silently or fail to load the module. Adding an empty Start() ensures compatibility:

function ClientInventoryService.Start()
end

Summary

Issue Severity Files Affected
SlotFactory button connections leak High SlotFactory.luau
BuildStorage search connection leaks Medium init.luau (client)
ConsoleDropUI button connections leak Medium ConsoleDropUI.luau
REPLICATE parameter casing Low init.luau (server)
GetUUidFromIndex naming typo Low InventoryStore.luau, call sites
Missing Start() function Low init.luau (client)
The connection leaks are the most impactful issue - in a production game with many slots, these create persistent memory growth over time.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions