Would this work, and how to improve it?

Help and Feedback Code Review
,
#1

This code is the code for a sword fight minigame.

How the minigame works: You get teleported to an arena, and after 10 seconds, you are given a sword. You kill everyone, and you get rewarded. More than 1 person alive and time ran out, it continues to lobby.

local module = require(game.ReplicatedStorage.GlobalFunctions)

game.ServerStorage.StartMinigame.Event:Connect(function(minigame)
	if minigame == "Sword Fight" then
		script.Parent.Sound:Play()
		local plrs = {}
		for i, p in pairs(game.Players:GetPlayers()) do
			local char = p.Character.HumanoidRootPart
			char.Position = script.Parent.SwordFightSpawn.Position + Vector3.new(math.random(-50.314 / 2, 50.314 / 2), 0, math.random(-49.3 / 2, 49.3 / 2))
			table.insert(plrs, p)
		end
		local newThread = coroutine.create(function()
			game.Players.PlayerRemoving:Connect(function(p)
				if (#plrs > 0) then
					local item = module.finditem(plrs, p)
					table.remove(plrs, item)
				end
			end)
			while (#plrs > 0) do
				for i, plr in pairs(plrs) do
					if plr.Character.Humanoid.Health <= 0 then
						plr.Backpack:ClearAllChildren()
						local item = module.finditem(plrs, plr)
						table.remove(plrs, item)
					end
				end
				wait(0.5)
			end
		end)
		coroutine.resume(newThread)
		for i = 10, 1, -1 do
			game.ReplicatedStorage.Text:FireAllClients("In "..tostring(i).." seconds, you will have to kill all opponents with a sword.")
			wait(1)
		end
		for i, pl in pairs(plrs) do
			local sword = game.ServerStorage.MinigameTools["1. Sword Fight Sword"]:Clone()
			sword.Parent = pl.Backpack
		end
		local val = 60
		repeat
			game.ReplicatedStorage.Text:FireAllClients("You have "..tostring(val).." seconds left to kill all opponents.")
			val = val - 1
			wait(1)
		until #plrs <= 1 or val == 0
		if #plrs <= 1 then
			if plrs[1] then
				game.ReplicatedStorage.Text:FireAllClients(plrs[1].Name.." has defeated all opponents!")
			end
			if #plrs == 0 then
				game.ReplicatedStorage.Text:FireAllClients("Somehow the last two contestants died at the same time, therefore no one has won.")
			end
		end
		if val == 0 then
			game.ReplicatedStorage.Text:FireAllClients("Time has ran out!")
		end
		for i, player in pairs(plrs) do
			local item = module.finditem(plrs, player)
			table.remove(plrs, item)
		end
		wait(3)
		script.Parent.Sound:Stop()
		game.ServerStorage.BeginIntermission:Fire()
		wait(1)
		script.Parent.Parent = game.ServerStorage.Minigames
	end
end)

How do I improve, and would this work?

#2

Why are you doing this? If it’s to keep the position inside of the boundaries, you can use a better approach.

1 Like
#3

Sorry if this information isn’t relevant, as this is a 2 day old post, but here’s some things I noticed:

		local newThread = coroutine.create(function()
			for i, plr in pairs(plrs) do
				plr.Character.Humanoid.Died:Connect(function()

You probably don’t need to create a new thread for this operation, as it’s relatively quick and events themselves are already asynchronous, making the benefits of creating a new thread minimal at best.

		if #plrs <= 1 then
			game.ReplicatedStorage.Text:FireAllClients(plrs[1].Name.." has defeated all opponents!")
		end

If we’re checking that the “plrs” table is less than one, then it’s possible that “plrs[1]” in the next line could throw an error for indexing a nil value. You should edit your if statement to avoid this issue.

		for i = 10, 1 -1 do

Unsure if this is a typo, but make sure you put a comma after the 1, (e.g. “for i = 10, 1, -1”)

Other than that, nothing really stuck out to me, so it’s possible I might have missed something. Keep up the good work!

3 Likes
#4

Oh, sorry, I updated my code. I’ll edit the original post once I get the chance.

edit: its been edited, and now you can see if there are any issues now.

also, sry if this counts as promotion, you can see the game and how it works here (it requires two players or more as well).

#5

Remember that CodeReview isn’t for seeing if your code works, you are to make sure it works.

1 Like
#6

It works, I’ll probably take down this topic because I do not need it anymore.

#7