I would appreciate any improvements you can find it my code!

So I’m making a Dice/Card game and I just finished with the dice rolling and it is acceptable, it works and looks mildly good, but it is also annoying how to rotates the die at the end and seems somewhat bumpy in places.

Before I give the code let me explain the parameters for the function. player is, well, the player. cSpawn is this:
Screenshot 2021-06-16 210909
The “Dice” folder will be filled of two of these dice:


The “Top” values are the Vector3 of the rotation of the Body of the dice when the parent side is facing up.

Mode isn’t used right now, ignore it.

Ok, now for the code. I tried to organize it and comment it to the best of my ability but it still seems a little messy. Sorry, I tried my best!

function actions.RollDie(player, cSpawn, Mode)
	print("Rolling dice")

	local Velocitys = {}

	--Turn on the body velocity and start lifting the dice
	for key, object in pairs(cSpawn.Dice:GetChildren()) do
		if object.PrimaryPart:FindFirstChild('BodyVelocity') then
			object.PrimaryPart.BodyVelocity.Velocity = Vector3.new(0,2,0)
		else
			local velocity = Instance.new("BodyVelocity",object.PrimaryPart)
			velocity.Velocity = Vector3.new(0,10,0)
			velocity.MaxForce = Vector3.new(4000,100000,4000)
			table.insert(Velocitys, #Velocitys + 1, velocity)
		end
	end

	--Move the camera for the players
	game.ReplicatedStorage.CameraEvents.TweenCamIn:FireAllClients(cSpawn.DiceCamPart)
	
	--Wait a bit for the camera to move and dice to rise
	wait(0.7)
	
	--Turn off the body velocity to stop lifting the dice
	for key, object in pairs(cSpawn.Dice:GetChildren()) do
		object.PrimaryPart.BodyVelocity.Velocity = Vector3.new(0,0,0)
	end

	--Roll all dice
	local Finished = 0
	for key, Die in pairs(cSpawn.Dice:GetChildren()) do
		local vel  = Instance.new('BodyAngularVelocity')
		vel.Parent = Die.PrimaryPart
		vel.AngularVelocity = Vector3.new(math.random(-360,360),math.random(-360,360),math.random(-360,360))
		vel.MaxTorque = Vector3.new(5000,5000,5000)
		game.ReplicatedStorage.playSound:FireAllClients(game.ReplicatedStorage.Sounds.DiceRolling, true)
		--This function changes the direction suddenly and tweens the dice to a still position
		spawn(function()
			wait(1)
			--Change velocity suddenly
			if vel.AngularVelocity.X > vel.AngularVelocity.Y and vel.AngularVelocity.X > vel.AngularVelocity.Z then
				--print("X is largest")
				vel.AngularVelocity = Vector3.new(vel.AngularVelocity.Y,vel.AngularVelocity.Z,vel.AngularVelocity.X)
			elseif vel.AngularVelocity.Y > vel.AngularVelocity.X and vel.AngularVelocity.Y > vel.AngularVelocity.Z then
				--print("Y is largest")
				vel.AngularVelocity = Vector3.new(vel.AngularVelocity.Z,vel.AngularVelocity.X,vel.AngularVelocity.Y)
			elseif vel.AngularVelocity.Z > vel.AngularVelocity.Y and vel.AngularVelocity.Z > vel.AngularVelocity.X then
				--print("Z is largest")
				vel.AngularVelocity = Vector3.new(vel.AngularVelocity.Y,vel.AngularVelocity.Z,vel.AngularVelocity.X)
			end

			wait(0.074)
			local tweenInfo = TweenInfo.new(
				1, -- Time
				Enum.EasingStyle.Linear, -- EasingStyle
				Enum.EasingDirection.Out, -- EasingDirection
				0, -- RepeatCount (when less than zero the tween will loop indefinitely)
				false, -- Reverses (tween will reverse once reaching it's goal)
				0 -- DelayTime
			)
			local Tween = TweenService:Create(vel, tweenInfo, {AngularVelocity = Vector3.new(0,0,0)})
			Tween:Play()
			wait(1)
			Finished = Finished + 1
		end)
	end
	--These functions help detect when the dice have stopped
	local function TakeCFrameReading(die)
		local CFrames = {}
		for key, side in pairs(die:GetChildren()) do
			if side.Name ~= "Body" and side:IsA("Part") then
				table.insert(CFrames, #CFrames + 1, side.CFrame)
			end
		end
		return CFrames
	end

	local function compareTables(table1, table2)
		local truthTable = {}
		for i = 1,#table1,1 do
			if table1[i] == table2[i] then
				table.insert(truthTable, i, i)
			end
		end
		if #truthTable == 6 then
			return true
		else
			return false
		end
	end
	
	--Returns true if the dice have stopped moving
	local function DetectRotation()
		print("Detecting movment")
		local FirstCFrame = nil
		local SecondCFrame = nil
		for key, die in pairs(cSpawn.Dice:GetChildren()) do
			FirstCFrame = TakeCFrameReading(die)
		end
		print("Got first reading")
		wait(0.1)
		for key, die in pairs(cSpawn.Dice:GetChildren()) do
			SecondCFrame = TakeCFrameReading(die)
		end
		print("Got second reading")
		if FirstCFrame and SecondCFrame then
			print("Got cframe tables")
			return compareTables(FirstCFrame,SecondCFrame)
		end
	end

	local HasStopped = nil
	--Wait untill the dice have reletivly stopped
	repeat HasStopped = DetectRotation() game:GetService("RunService").Heartbeat:Wait() until Finished == #cSpawn.Dice:GetChildren() and HasStopped == true

	--Calculate the top sides and tween them so they are more clear
	for key, Die in pairs(cSpawn.Dice:GetChildren()) do
		local TopSide = nil
		local SidePositions = {}
		
		--Adds all the side Y positions
		for i = 1,#Die:GetChildren() do
			local foundChild = Die:FindFirstChild('Side'..i)
			if foundChild and foundChild:IsA('BasePart') then
				SidePositions[#SidePositions + 1] = foundChild.Position.Y
			end
		end
		
		if SidePositions then
			print(SidePositions)
			if #SidePositions == 6 then
				local TopSide
				for i = 1,#SidePositions do
					local Current = SidePositions[i]
					local Number = 0
					for x = 1,#SidePositions do
						if SidePositions[x] ~= Current then
							if Current > SidePositions[x] then
								Number = Number + 1
							end
						end
					end
					if Number == (#SidePositions - 1) then -- We check for -1 because the value we are checking is also part of the table
						TopSide = Die:FindFirstChild("Side" .. i)
					end
				end
				
				--Tween the dice to a clear position
				spawn(function()
					local tweenInfo = TweenInfo.new(
						0.2, -- Time
						Enum.EasingStyle.Quart, -- EasingStyle
						Enum.EasingDirection.Out, -- EasingDirection
						0, -- RepeatCount (when less than zero the tween will loop indefinitely)
						false, -- Reverses (tween will reverse once reaching it's goal)
						0 -- DelayTime
					)

					local tween = TweenService:Create(Die.PrimaryPart, tweenInfo, {Orientation = TopSide.Top.Value})

					tween:Play()
					wait(0.5)
					--Pay the ding sound
					game.ReplicatedStorage.playSound:FireAllClients(game.ReplicatedStorage.Sounds.ding, true)
					wait(0.7)
					--Tween cam out and destroy body velocitys
					game.ReplicatedStorage.CameraEvents.TweenCamOut:FireAllClients()
					Die.PrimaryPart.BodyVelocity:Destroy()
					Die.PrimaryPart.BodyAngularVelocity:Destroy()
				end)
			end
		end
	end
end

Yeah, I know, that is a whole lot of code. I understand that you may not want to ready aaaaaalll of that but I would really really if you took the time to help me out. Here is a video showing the result which, as I mentioned earlier, is great, but not perfect.

Code Review Result.wmv (1.1 MB)

Honestly, this code looks pretty good. Good job on the commenting, and nothing really seems that bad. You don’t seem to repeat your self a lot, and you are using functions and all that correctly. Probably could improve DetectRotation a little bit though as it does repeat just a little bit. And in compareTables you can change it to something like this:

local function compareTables(table1, table2)
		local truthTable = {}
		for i = 1,#table1,1 do
			if table1[i] == table2[i] then
				table.insert(truthTable, i, i)
			end
		end
		if #truthTable == 6 then
			return true
		end
        return false;
	end

Other than that, good job!!!

1 Like