I personally always use Getters and Setters in classes in Roblox because it makes refactoring a lot easier. However, this can be really cumbersome to modify data. However, in this case, the way you design your objects can make this a bit cleaner.
In this case, your question brings up an important point–this design feels clunky, but I don’t think it’s because of the Getters and Setters.
Here’s my thought process:
-
First thought: Slots probably shouldn’t know about inventory. If they need to know about it (i.e. some consumer of slots needs the inventory), you should just be passing the whole inventory around, since clearly the consumer of the slots is really manipulating the whole inventory–not just a single slot. Furthermore, I suspect there’s very few cases where you want to isolate a single slot from the rest of the inventory in terms of refactoring.
-
Second thought: Slots in this case are little more than data containers. The fact that the Inventory has a method called
OnSlotChanged implies that the inventory isn’t benefitting at all from the encapsulation of slots, i.e. slots aren’t really decoupled from the inventory object at all. Furthermore, the fact that the inventory has a method called GetInventoryData implies that the slots arne’t even the owner of their own data! This implies to me you should simplify your design to just an inventory object.
In exploring this thought, we should consider what your inventory system might be in charge of:
- Source of truth for items stored in inventory
- Exposing changes to the inventory to the UI
- Serialization of data (this should be separated from the inventory object)
- Synchronization of data between client and server.
In this case, you probably want the server to be the source of truth. The single responsibility of the inventory in this case should be to thus be a holder of data.
I just spent like 30 minutes trying to make the OOP feel better, and it turns into a sort of messy solution. You sort of end up with a ton of objects to hold the slot contents, even though I think they’re sort of just filler.
Thinking about this problem more, you probably can get away with an immutable inventory as the source of truth. This leads to a solution like Rodux or Roact. I gave that a shot, and end up with something a bit cleaner.
I think since this problem is fundamentally about manipulating data, and not about behavior of slots (which tend to be mostly the same), then OOP is less useful in this case.
---
-- @module Inventory
-- @author Quenty
local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))
local Rodux = require("Rodux")
local SlotReducer = require("SlotReducer")
return Rodux.combineReducers({
food = SlotReducer(10, "food");
potions = SlotReducer(5, "potion");
})
---
-- @module SlotReducer
-- @author Quenty
local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))
local Rodux = require("Rodux")
local ItemReducer = require("ItemReducer")
return function(numOfSlots, slotType)
assert(numOfSlots)
assert(slotType)
return function(state, action)
if action.slotType ~= slotType then
return state
end
assert(action.slotNumber)
local newState = {}
for i=1, numOfSlots do
if i == slotNumber then
newState[i] = ItemReducer(state[i] or {}, action)
else
newState[i] = state[i] or {}
end
end
return newState
end
end
And then to dispatch you’d do something like
rodux:dispatch({
type = "setSlotContents";
slotType = "food";
slotKey = "slotKey";
contents = { }; -- TODO: Put in item data for stack here!
})
---
-- @module ItemReducer
-- @author Quenty
local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))
local Rodux = require("Rodux")
return Rodux.createReducer({}, {
setSlotContents = function(state, action)
return {
contents = action.contents;
}
end;
clearSlotContents = function(state, action)
return {}
end;
})
tl;dr Maybe in this case, OOP isn’t the correct solution