My data keeps duplicating itself for unknown reason

Ninja_Doge097 · January 4, 2025, 12:50pm

Recently, I encountered a very strange bug that seems to duplicating my items whenever I join the game.

So I have those 2 tables named savedItems and SESSION_TABLE.

The main differences between them is that savedItems is saved to the datastore, while SESSION_TABLE is not. The purpose of SESSION_TABLE is to basically have savedItems table assigned to it and whenever an item is picked up by the player, the SESSION_TABLE is going to write down the data about an item that has to be saved in savedItems e.g. its position in player’s inventory, display name.

When player leaves the game, right before the data is saved, savedItems table will get all the data about the items from the SESSION_TABLE and assign it for itself.

(For your information, I’m using ProfileService to save data)

My problem is that the items somehow duplicate itself multiple times when the player owns one. (I’m using HttpService:GenerateGUID() to generate unique identifiers for items so they dont override eachother when player has multiple items of the same kind)

I’ll give an example:
I start the game with no items. During my progression, I find a Sword. When I leave the game, my Sword is saved to savedItems table. Here’s how it looks like:

But when I join back, my Sword duplicates itself for horrendous amounts for unknown reason:

Here are fragments of 2 scripts:

local SS = game:GetService("ServerStorage")
local RS = game:GetService("ReplicatedStorage")

local ProfileService = require(script.Parent.ProfileService)
local InventoryService = require(script.Parent.InventoryService)

local ProfileTemplate = {
	savedItems = {},
	money = 0
}

local ProfileStore = ProfileService.GetProfileStore("PlayerData",ProfileTemplate)

local players = {}

local PlayerService = {}
local Profiles = {}

function onPlayerAdded(plr)
	local profile = ProfileStore:LoadProfileAsync("Player_" .. plr.UserId)
	if profile ~= nil then
		profile:AddUserId(plr.UserId)
		profile:Reconcile()

		profile:ListenToRelease(function()
			Profiles[plr] = nil
			plr:Kick()
		end)
		
		if plr:IsDescendantOf(game.Players) then
			Profiles[plr] = profile
		else
			profile:Release()
		end
	else
		plr:Kick() 
	end
end

function PlayerService.start()
	for i,v in game.Players:GetPlayers() do
		task.spawn(onPlayerAdded, v)
	end

	game.Players.PlayerAdded:Connect(function(plr)
		onPlayerAdded(plr)
		players[plr.UserId] = {}

		local PlayerInfo = players[plr.UserId]
		local PlayerData = PlayerService.GetData(plr)
		local PlayerInventory = InventoryService.GetPlayerInventory(plr)

		PlayerInfo.SESSION_TABLE = PlayerData.savedItems
		
		for i,v in pairs(PlayerInfo.savedItems) do
			PlayerInventory:AddTool(v.Name)
		end

	end)

	game.Players.PlayerRemoving:Connect(function(plr)
		local PlayerData = PlayerService.GetData(plr)
		
		PlayerData.savedItems = players[plr.UserId].SESSION_TABLE

		players[plr.UserId] = nil
		local profile = Profiles[plr]
		if profile ~= nil then
			profile:Release()
		end
	end)

end

function PlayerService.GetList()
	return players
end

function PlayerService.GetPlayer(plr)
	return players[plr.UserId]
end

function PlayerService.GetData(plr)
	local profile = Profiles[plr]
	if profile ~= nil then
		return profile.Data
	end
end

function PlayerService.GetProfile(plr)
	local profile = Profiles[plr]
	if profile ~= nil then
		return profile
	end
end

return PlayerService

local MaxHotbarTools = 5

local HotbarKeys = {
	[1] = "One",
	[2] = "Two",
	[3] = "Three",
	[4] = "Four",
	[5] = "Five",
}

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local ServerStorage = game:GetService("ServerStorage")
local HttpService = game:GetService("HttpService")

local Networking = ReplicatedStorage.Inventory.Networking
local Util = ReplicatedStorage.Inventory.Util

local InventorySystem = script.Parent.Parent
local ToolSystem = InventorySystem.ToolSystem

local Loader = require(Util.Loader)
local ToolClasses = Loader.LoadDirectory(ToolSystem.Tools, true)
local PlayerService = require(script.Parent.Parent.Parent.PlayerService)

local InventoryBase = {}
InventoryBase.__index = InventoryBase

function InventoryBase.new(player)
	local self = setmetatable({
		Tools = {
			Hotbar = {},
			Backpack = {},
			EquippedTool = nil
		},

		Player = player,
		NumberOfTools = 0,
		ToolsInHotbar = 0,
		LastId = 0
	}, InventoryBase)

	return self
end

function GenerateGUID()
	return HttpService:GenerateGUID(false)
end

function InventoryBase:AddTool(toolName)
	if (not toolName or type(toolName) ~= "string") then return end
	if (not ToolClasses[toolName]) then return end

	if (self.ToolsInHotbar < MaxHotbarTools) then
		self.NumberOfTools += 1
		self.ToolsInHotbar += 1
		self.LastId += 1
		
		-- // This is going to be added to player's inventory with all the tool's functions and other stuff

		local toolKey = HotbarKeys[self.NumberOfTools]
		self.Tools.Hotbar[toolKey] = ToolClasses[toolName].new()

		local newTool = self.Tools.Hotbar[toolKey]

		newTool.EquipKey = toolKey
		newTool.ID = self.LastId 
		newTool.Name = toolName
		
		-- // This is going to hold the most important data about the tool such as its name, position in inventory etc.

		local identifier = GenerateGUID()
		local PlayerInfo = PlayerService.GetPlayer(self.Player)
		PlayerInfo.SESSION_TABLE[identifier] = {}

		local data = PlayerInfo.SESSION_TABLE[identifier]

		data.Name = toolName
		data.positionId = self.LastId
		
		-- // Then the client does the rest of the code (The data and the session table isn't affected by client in any means)
		Networking.RE.ToolAdded:FireClient(self.Player, {Name = toolName, Key = toolKey, ID = newTool.ID})
	end
end

return InventoryBase

What I find really strange is how this fragment iterates itself multiple times even if the savedItems table clearly has 1 item saved:

for i,v in pairs(PlayerInfo.savedItems) do
	PlayerInventory:AddTool(v.Name)
end

I’m deeply sorry if I explained my issue poorly

bura1414 · January 4, 2025, 2:18pm

whats a point of these ? this just make data harder to work on imagine you need to check if player has specific item but because of that you hae to loop through each item and check name also longer the name = more space

probably issue with data loading or saving you can just add print statements and see if it properly saves or loads data and check if theres anything unusual going on

Ziffixture · January 4, 2025, 3:02pm

What’s not working

It seems like you store tools under a GUID that maps to a name an a position in the inventory. However, each time you return the tool to the inventory, you assign it a new GUID, which effectively makes it a new instance. This isn’t the root of the problem though. I believe you misunderstand these lines of code for duplicating tables:

PlayerInfo.SESSION_TABLE = PlayerData.savedItems

PlayerData.savedItems = players[plr.UserId].SESSION_TABLE

The above lines of code do not duplicate the respective tables. Tables, among very few things in Lua(u), are passed by reference, so by assigning PlayerInfo.SESSION_TABLE to PlayerData.savedItems, you’re just making PlayerInfo.SESSION_TABLE an alias to PlayerData.savedItems. Any changes you make to the session table will actually be made to the saved items table:

local x = {} -- table: 0x000000000002f150
local y = x

y.z = "Hello, world!"

print(x.z) --> Hello, world!
print(y) -- table: 0x000000000002f150

Because your inventory class writes these tools to the session data with new GUIDs, you are effectively adding new (copied) tools to the saved data table.

What you can do about it

Keep your session data separate from your saved data, or eliminate session data altogether as your current implementation makes it redundant—profiles are already a form of session data.

If you’re going to use GUIDs, do not generate them on the fly. Each item in your game should have a fixed GUID set in Roblox Studio. The only reason for doing this is so instances can be identified outside of their name, which may be subject to change.
Modify your function to only accept GUIDs and the instance itself for sourcing a GUID. Check if the GUID is not present in the player’s saved items before attempting to update the saved items. It’s at this point you could also implement item stacking.

Additionally, your inventory service class should be the one to load the player’s items, not your data persistence system

Ninja_Doge097 · January 4, 2025, 5:49pm

You are right. That explains why whenever and whatever gets added to my session table, it also shows up on the savedItems table.

I managed to do that by simply iterating through savedItems table and assigning its indexes and values to the session table.

for i,v in pairs(PlayerData.savedItems) do
	playerInfo.SESSION_TABLE[i] = v
end

Although I’m not sure if that’s a really effective and convenient way to do that, but it gets the job done.

And I was able to defeat the duping problem by just writing a function, similiar one to Inventory:AddTool(name) but all I did here was remove a snippet where it registers item’s data and assigns it to the session table and can be only used during the loading process:

function InventoryBase:LoadTool(toolName)
	if (not toolName or type(toolName) ~= "string") then return end
	if (not ToolClasses[toolName]) then return end

	if (self.ToolsInHotbar < MaxHotbarTools) then
		self.NumberOfTools += 1
		self.ToolsInHotbar += 1
		self.LastId += 1

		local toolKey = HotbarKeys[self.NumberOfTools]
		self.Tools.Hotbar[toolKey] = ToolClasses[toolName].new()

		local newTool = self.Tools.Hotbar[toolKey]

		newTool.EquipKey = toolKey
		newTool.ID = self.LastId 
		newTool.Name = toolName

		Networking.RE.ToolAdded:FireClient(self.Player, {Name = toolName, Key = toolKey, ID = newTool.ID})
	end
end

May not seem like the best solution here, but I’m still very new to saving inventories and even making one from scratch. So I’m sure there are probably better and more comfortable ways to do that, but it’s least of my concerns as of right now.

But I’m not quite sure on how I could identify each tool with their own data.

But it’s not like each tool of the same kind is going to have the exact same data like the others e.g. 2 swords may be tools of the same kind, but both of them have different durabilities.

Ziffixture · January 4, 2025, 7:08pm

Item-stacking / individual instances of same type

If two items cannot be stacked, then that’s that. If they can, then you include a quantity property in the data. For items of the same type but with different data, you can map the item type GUID to an array of each serialized item of that type. This contradicts the second pointer I gave to you regarding checking against existing data, and will requirement an amendment to that rule.

Your solution

As for implementing a separate function for loading the player’s existing items, that is what I meant by

“Additionally, your inventory service class should be the one to load the player’s items, not your data persistence system”

It is an optimal solution, however, it could be improved through the use of function-chaining as you’re currently duplicating your code. You can modify both your AddTool and LoadTool function to return a boolean representing whether or not the operation was successful. This is useful information for any calling code of those functions. Have AddTool call LoadTool and only proceed to save the item if LoadTool was successful.

function InventoryBase:AddTool(tool: Tool | string): boolean
    local success = InventoryBase:LoadTool(tool)
    if not success then
        return false
    end

    -- Save tool

    return true
end

The final vision I have for this solution is to implement a third and final function called “LoadSavedTools”. This will be responsible for accessing the given player’s profile and calling LoadTool on each item. Your data persistence system should be ignorant of where its data goes and how it’s used throughout your codebase; it should only be concerned with managing and loading profiles. This is the basis of de-coupling and the single-responsibility principle (SRP)

Session data

“Although I’m not sure if that’s a really effective and convenient way to do that, but it gets the job done.”

What you have is called a shallow copy, and will only ensure your session data table does not act as an alias for the stored data table. If any reference values in the stored data table exist, however, those will still carry over to the shallow copy, and changes to those values in the shallow copy will reflect on the original. To completely avoid this, you need a recursive deep-copy algorithm. If this isn’t a concern to you, you can use table.clone to shorten your code.

Once again though, if all you do is trade your stored data for your session data at the end of the player’s time in the server, then the creation of this session data is completely redundant, and you should continue to work solely on the stored data table

system · January 18, 2025, 7:08pm

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.