Help with my script

Help and Feedback Code Review
3

I see a couple of small issues in the script. The version I’m sending is tested. I hope you don’t mind that I rewrote the script according to the Roblox Lua Style guide. If you want, you can remove the brackets again, though I suggest you follow the guide for the sake of other people possibly reading your code.

  • Parent property is assigned last (performance reasons: PSA: Don't use Instance.new() with parent argument).

  • All modern Roblox’s method names are capitalized (e.g. :Connect() ).

  • Avoid yielding the script to fix any issues with loading.

  • As @vopwn55 mentioned, FindFirstChild() won’t wait for the object to load.

  • ROOT PROBLEM: script.Parent.Humanoid.

Your script was placed in StarterCharacterScripts and got copied into every new character. That’s why connections didn’t behave as expected (connecting too late and maybe even accumulating at times).

Firstly, the script is modifying old humanoid’s properties, and secondly, this server script belongs in ServerScriptService. If you wanted this script in every character - which I don’t encourage - you should get rid of PlayerAdded and CharacterAdded altogether.

I thought an easy way to solve this was having a table to store maximal healths for each player.

  • I see you’re using the creator tag (typical for Classic swords). Perhaps use a unique name so that humanoid search without loops is possible, for instance
local killerTag = humanoid:FindFirstChild("Creator")
  • When MaxHealth is reduced, Health can still remain the same (bug?).

Now, let me summarize how I understand your script.

  1. When Player1 kills Player2, Player1 grows MaxHealth by 10, and Player2’s MaxHealth is decreased by 10. Player1’s kills incrase by 1, and Player2’s death count increases by 1.

  2. When Player dies, their MaxHealth is decreased by 10, and their death count is increased by 1.

Following this logic, here’s your script.

local Players = game:GetService("Players")

local DEFAULT_HEALTH = 100
local MINIMAL_MAX_HEALTH = 10

local template = Instance.new('Folder') do
	template.Name = 'leaderstats'
	local kills = Instance.new('IntValue')
	kills.Name = 'Kills'; kills.Parent = template
	local deaths = Instance.new('IntValue')
	deaths.Name = 'Deaths'; deaths.Parent = template
end

local playerMaxHealth = {}

Players.PlayerAdded:Connect(function(player)
	local leaderstats = template:Clone()
	leaderstats.Parent = player
	
	local kills = leaderstats.Kills
	local deaths = leaderstats.Deaths
	
	local currentCharacter
	
	playerMaxHealth[player] = DEFAULT_HEALTH
	
	player.CharacterAdded:Connect(function(character)
		if currentCharacter then currentCharacter:Destroy() end
		currentCharacter = character
		
		local humanoid = character:WaitForChild('Humanoid')
		humanoid.MaxHealth = playerMaxHealth[player]
		humanoid.Health = playerMaxHealth[player]
		
		humanoid.Died:Once(function()
			deaths.Value += 1
			
			if playerMaxHealth[player] -10 < MINIMAL_MAX_HEALTH then
				playerMaxHealth[player] = MINIMAL_MAX_HEALTH
			else
				playerMaxHealth[player] -= 10
			end
			
			for _, child in ipairs(humanoid:GetChildren()) do
				if child:IsA('ObjectValue') and child.Value and child.Value:IsA("Player") then
					
					local killer = child.Value
					killer.leaderstats.Kills.Value += 1 -- definitely loaded
					playerMaxHealth[killer] += 10
					
					local killerHumanoid = killer.Character and killer.Character:FindFirstChild("Humanoid")
					killerHumanoid.MaxHealth = playerMaxHealth[killer]
					
					break;
				end
			end
		end)
	end)
end)
2 Likes