Help with making my random item generator cleaner?

Help and Feedback Scripting Support
#1

In my game, I tried making it so when you respawn after dying, random items will be cloned into your backpack. At the moment, it works but it’s not as clean as I’d like it to work. The player’s character would respawn a bunch of times after they die. I’ve tried 2 different ways, I checked if only the player’s character was added or respawned, and if the player has died and got the player respawn time and cloned the items, both of which just cloned the items so many times the game just crashes. Then I did both check if the player’s character was added and check if it’s died.

for x = 300,0,-1 do

	for j,plr in pairs(plrs) do
		if plr then
			if plr.Character then
				-- they in the game

				plr.Character.Humanoid.Died:Connect(function()
					wait(game.Players.RespawnTime)
					local weapons = game.ServerStorage:WaitForChild("weapons")

					local melee = weapons:WaitForChild("Melee"):GetChildren()
					local ranged = weapons:WaitForChild("Ranged"):GetChildren()
					local misc = weapons:WaitForChild("Misc"):GetChildren()


					local chosenMelee = melee[math.random(1,#melee)]:Clone()
					local chosenRanged = ranged[math.random(1,#ranged)]:Clone()
					local chosenMisc = misc[math.random(1,#misc)]:Clone()

					chosenMelee.Parent = plr.Backpack
					chosenRanged.Parent = plr.Backpack
					chosenMisc.Parent = plr.Backpack

					print(plr.Name,chosenMelee.Name,chosenRanged.Name,chosenMisc.Name)

				end)

			end
		else
			print(plrs[j].Name..' was removed')
			table.remove(plrs,j)
		end
	end

	
	wait(1)
	if x == 0 then
		--end of game
		wait(2)
		break
	end

end
1 Like
#2

plrs is a table with all the players in the game in it!

#3

I guess you could create a remote event that controls the cloning of items.

1 Like
#4

Why are you doing this 300 times

#5

It’s a timer that counts down from 300…

#6

Take the Humanoid.Died event and the plrs loop out of the timer and your game shouldn’t crash anymore.

#7

Then how would I give the player random items?

#8

Replace Humanoid.Died with CharacterAdded event.

local Players = game:GetService("Players")

Players.PlayerAdded:Connect(function(Player)
     Player.CharacterAdded:Connect(function()
     -- clone items into backpack
     end)
end)
#9

I’m confused. Do you want the player to be given a new item every second? I’m not sure why you’re creating 300 connections.

Anyway, I think you’re using multiple bad practices so here’s a bit of advice for improving your code:

1- Use the players.PlayerAdded and the player.CharacterAdded events
2- Use :GetService on your services
3- You should define your constants at the top of your scripts as :WaitForChild is slightly slower than :FindFirstChild
4- You don’t have to blatantly check if your loop is over, you can just add whatever happens at the end of the round at the end after the loop is over

local players = game:GetService('Players')
local serverStorage = game:GetService('ServerStorage')

local weapons = serverStorage:WaitForChild('weapons')
local melee = weapons:WaitForChild('Melee'):GetChildren()
local ranged = weapons:WaitForChild('Ranged'):GetChildren()
local misc = weapons:WaitForChild('Misc'):GetChildren()

players.PlayerAdded:Connect(function(player)
    local function handleRespawn(character)
        local backpack = player:WaitForChild('Backpack')
        local chosenMelee = melee[math.random(1,#melee)]:Clone() -- if you want to shorten it even more, you could condense it to 'melee[math.random(1,#melee)]:Clone().Parent = backpack'
        local chosenRanged = ranged[math.random(1,#ranged)]:Clone() -- ranged[math.random(1,#ranged)]:Clone().Parent = backpack
        local chosenMisc = misc[math.random(1,#misc)]:Clone() -- misc[math.random(1,#misc)]:Clone().Parent = backpack
        chosenMelee.Parent = backpack -- if you're condensing the 3 lines above this one, you can remove these parent assignments
        chosenRanged.Parent = backpack
        chosenMisc.Parent = backpack
    end
    local character = player.Character or player.CharacterAdded:Wait()
    handleRespawn(character)
    player.CharacterAdded:Connect(handleRespawn)
end)

for i = 300,0,-1 do
    wait(1)
end

-- you could do whatever happens at the end of the game here
2 Likes
#10

I was going for a Randomizer or Item Asylum type game, if you’ve played either of those, after every time you die when the round is in progress, you’re given different items each time. I’ll try this though :grin:

1 Like
#11

You have Humanoid.Died connecting 300 times

#12

This is a typical setup for running code when a player joins:

local function onPlayerAdded(player)
   -- anything you want to happen when the player joins you put
   -- in here including onCharacterAdded
end

-- this ensures onPlayerAdded runs for players that joined before this script ran
for _,player in ipairs(game.Players:GetPlayers()) do
     task.spawn(onPlayerAdded)(player)
end

-- connect onPlayerAdded to every new player
game.Players.PlayerAdded:Connect(onPlayerAdded)
#13

Just wanna add that iterating through the players is only necessary if the code above the connection is asynchronous (yields).

#14

players.CharacterAdded isn’t working :confused:

#15

Shoot, I meant to do players.PlayerAdded, not .CharacterAdded, my bad. I’ll edit the post.

Edit: done

#16

That is not true, the point of iterating is in case a player joins before the code runs.

I think what you meant is the point of task.spawn() is only if the above code yeilds

#17

It’s okay :laughing: But when I went to play test, I reset and it didn’t do anything. I’m assuming because other players hasn’t joined the game game.
Edit: I put players.PlayerAdded as well

#18

I’m confused. I’m pretty sure scripts in ServerScriptService run before the player is added. I’ve never seen an instance of the script running after the player is added.

Also @jorcorrs, I made an edit to the post right after you posted your initial reply, Ensure you have these 3 lines:

    local character = player.Character or player.CharacterAdded:Wait()
    handleRespawn(character)
    player.CharacterAdded:Connect(handleRespawn)

If it still isn’t working, are there any output errors?

#19

Here’s a video from sleitnick explaining it:

Edit: I just realized you were talking about the code above the for loop yielding I thought you were talking about the code in onPlayerAdded yielding, my error

#20

Right, he’s adding that iteration because of the wait(2) which yields the thread before the .PlayerAdded connection is made.