Help with my script

Help and Feedback Code Review
#1

Im trying to make a game where when you get a kill you gain max health and the opposite happens when you die.
Code:

local Players = game.Players

local Template = Instance.new 'BoolValue'
Template.Name = 'leaderstats'

Instance.new('IntValue', Template).Name = "Kills"
Instance.new('IntValue', Template).Name = "Deaths"


Players.PlayerAdded:connect(function(Player)
	wait(1)
	local Stats = Template:Clone()
	Stats.Parent = Player
	local Deaths = Stats.Deaths
	Player.CharacterAdded:connect(function(Character)
		Deaths.Value = Deaths.Value + 1
		script.Parent.Humanoid.MaxHealth -= 10
		local Humanoid = Character:FindFirstChild "Humanoid"
		if Humanoid then
			Humanoid.Died:connect(function()
				for i, Child in pairs(Humanoid:GetChildren()) do
					if Child:IsA('ObjectValue') and Child.Value and Child.Value:IsA('Player') then
						local Killer = Child.Value
						if Killer:FindFirstChild 'leaderstats' and Killer.leaderstats:FindFirstChild "Kills" then
							local Kills = Killer.leaderstats.Kills
							Kills.Value = Kills.Value + 1
							script.Parent.Humanoid.MaxHealth += 10
						end
						return -- Only one player can get a KO for killing a player. 
					end
				end
			end)
		end
	end)
end)

I tested it and it didnt work .The leaderboard worked but the max health didnt change.

1 Like
#2

Does this line output any error in the console? This seems terribly wrong to me.

In addition, is the task.wait(1) at the beginning necessary? Because if the player spawns in less than a second, the CharacterAdded event won’t fire the first time, potentially breaking your script.

And also, replace FindFirstChild with WaitForChild with a timeout, because it will wait some time in case the Humanoid appears in the future.

3 Likes
#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
#4

Will this work? And where should i put it.

#5

Yes, it has worked flawlessly in my tests. Server script in ServerScriptService.

#6

THANKS! Also, how would you make it ban the player for 1 hour if he loses all his health .

#7

You can use os.time() (you can read about it here) to determine whether the player should be banned or not, if they die then you can assign the time to a variable that’s saved with a datastore, you can look up on the internet how to make a datastore and you can add some of your own code to it in order to achieve what you’re looking for.