How Could I Improve A NumberValue Saving System

Help and Feedback Code Review
#1

I’m new to scripting and just wanted to know how this code could be improved. One part I’m concerned about is when I loop through upgrade_folder. There’s only one child of the folder which is a number value. Is there a better way I could do instead of a for loop? Please also give me feedback on the rest of the code, thanks.

local ds = game:GetService("DataStoreService"):GetDataStore("Upgrade_saving__47859bv9nwvg")
local upgrade_folder = game.ServerStorage.Upgrades -- Change this to where you're storing the tools

game.Players.PlayerAdded:Connect(function(plr)
  local item_fold = Instance.new("Folder")
  item_fold.Name = "upgrades"
  item_fold.Parent = plr

  for _, v in pairs (upgrade_folder:GetChildren()) do -- Creates a new bool value for every tool in the folder
    local upgrade = Instance.new("NumberValue")
    upgrade.Name = v.Name -- Setting the bool value name to the weapon's name
    upgrade.Parent = item_fold
    upgrade.Value = 1
    if ds:GetAsync(v.Name .. plr.UserId) ~= nil then -- Checking to see if the player has ever played the game
      local succ, msg = pcall(function()
        upgrade.Value = ds:GetAsync(v.Name.. plr.UserId) -- setting the value to the saved data
      end)
      if not succ then
        warn("Problem with getting and setting data "..msg)
      end

    end
  end

end)

game.Players.PlayerRemoving:Connect(function(plr)
  local succ, msg = pcall(function()
    for _, v in pairs (plr.items:GetChildren()) do
      ds:SetAsync(v.Name .. plr.UserId, v.Value) -- Saving the values
    end
  end)

  if not succ then
    warn("Problem with saving data ".. msg)
  end
end)
#2

The biggest concern that sticks out to me is storing the data to the player with a folder. You should almost always refrain from giving the client power to manipulate game data. Storing integer values in a folder parented to the player and grabbing from the same point later to save would make it criminally easy for the client to exploit data.

I would consider storing client data to a table in a server script to save yourself from an exploiting disaster. Also, have you heard of DataStore2?

#3

Try using the BindToClose function. It can help to save data when the game is shutting down.

Also, try to use UpdateAsync rather than SetAsync. UpdateAsync can prevent datastore failures most of the time.