-
Notifications
You must be signed in to change notification settings - Fork 1
Fix connection leaks, naming conventions, and missing Start() function #2
Description
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 cleanupConsoleDropUI.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 caseGetUUidFromVisibleHotbarIndex- mixed case
Fix: Rename to:GetUUIDFromIndexGetUUIDFromVisibleHotbarIndex
Update all call sites accordingly (e.g., ininit.luauandInventoryController.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()
endSummary
| 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. |