Feedback on inventory sytem using Knit

[IamACoolGuy_1230 (Scorch)]

Hey there!

I’ve recently been trying to get into Knit framework, and to start off I’ve created a simple inventory system.

There are 2 services: PlayerService and InventoryService.

Player Service handles players joining, creating and loading data etc.
InventoryService handles adding items to player’s inventory, removing items, etc.

When a player joins, PlayerService calls CreatePlayerData(), and the resulting table is stored in a PlayerData module, which can be accessed by other scripts.

When InventoryService:Add() is called, it uses a method in PlayerService to get the player’s data, and edits it with the new item.

So my questions are:

1. Is the way PlayerService handles and stores player data a good practice

2. Is the way InventoryService adds items to the inventory good practice

3. And mainly: is the way I have structured my game and its codebase good practice and good for maintainability further down the line

Here is a link to the github repo: GitHub - cadpil/Utilities
(All services and scripts are in src/ServerStorage)

I appreciate any and all feedback, as I just want to make sure that the way I have layed this out isn’t going to come back to haunt me because it’s unmaintainable.

[UIScript]

At a glance, it looks pretty good and readable, almost exactly how I would do it myself, as long as you are comfortable with it.

@IamACoolGuy_1230

Is the way PlayerService handles and stores player data a good practice

Yeah, I use pretty much the same structure when storing data (incorporating ProfileService into mine, but use what you want).

@IamACoolGuy_1230

Is the way InventoryService adds items to the inventory good practice

My only concern with this is you are always assuming your player data variables will always return something. If you dont want to deal with the hassle of checking if it exists on every function, I would recommend adding a GetPlayers and/or HasData functions in your PlayerService to only return an array of players with a session data, along with making sure your functions are being called asynchronously to prevent any edge cases. i.e:

-- Finish chopping a tree
local contributingPlayers = {} -- table of players who contributed to something
for _, player in ipairs(contributingPlayers) do
    if PlayerService:HasData(player) then -- in case a player ends up leaving before/during the loop
        task.spawn(function() -- ^
            InventoryService:Add(...)
        end)
    end
end

-- Give everyone in the server a free item
for _, player in ipairs(PlayerService:GetPlayers()) do
    task.spawn(function()
        InventoryService:Add(...)
    end)
end

@IamACoolGuy_1230

And mainly: is the way I have structured my game and its codebase good practice and good for maintainability further down the line

Yeah. There are really no right or wrong ways with how you structure your game, as long as you are comfortable with it. It’s organized, readable, and very easy to expand upon.