Why do the blocks double every time?

Adeenex · March 8, 2020, 11:19pm

im new to coding btw the issue that i’m having is that the blocks keep doubling. I don’t know what the problem is, but i think it is with this script, as the other stop placement one works without doubling it.

 local function LowBlocks()
	if game.ReplicatedStorage.Values.GSOoneblock.Value <= 0 and IsPlacing and CanPlace and not CanStart then
		local yeetele = Player.PlayerGui.ScreenGui.MainGui
		Keyz:TweenPosition(UDim2.new(0.426, 0,1, 0),"Out","Quad",0.4)
		Keyzz:TweenPosition(UDim2.new(0.277, 0,1, 0),"Out","Quad",0.4)
 		Keyzzz:TweenPosition(UDim2.new(0.567, 0,1, 0),"Out","Quad",0.4)

		IsPlacing = false
		CanPlace = false
		CanStart = true
		Isyes = true
		
		yeetele.GSOoneblockbut.Visible = false
		eventdelete:FireServer()
	else
		local yeetele = Player.PlayerGui.ScreenGui.MainGui
		yeetele.GSOoneblockbut.Visible = true
	end
end

and on the server side it is just Model:Destroy().
The server is creating the block too.

IdiomicLanguage · March 8, 2020, 11:26pm

If the server is creating the block too then the issue is not here in the client code. I’m also not sure how the code you gave fits into your project but I don’t see anything that should cause this issue.

ShadokuSan · March 8, 2020, 11:26pm

With what is shown here, there is not enough to go off on in helping you find the exact solution. We’d have to see the actual code that processing making the blocks themselves, or more of the local script you’re using to send messages to the server. There’s likely some part that is making multiple connections where it shouldn’t be.

Adeenex · March 8, 2020, 11:27pm

client script:

local RS = game:GetService("RunService")
local UIS = game:GetService("UserInputService")

local Player = game.Players.LocalPlayer
local Char = Player.Character or Player.CharacterAdded:Wait()
local Mouse = game.Players.LocalPlayer:GetMouse()

local RSe = game:GetService("ReplicatedStorage")
local Player = game.Players.LocalPlayer
local event = RSe.Events:WaitForChild("PlacementEvent")
local event2 = RSe.Events:WaitForChild("Move")
local event3 = RSe.Events:WaitForChild("Stop")
local eventdelete = RSe.Events:WaitForChild("NoN")
local eventdestroy = RSe.Events:WaitForChild("No")
local eventstart = RSe.Events:WaitForChild("Yes")
local event4 = RSe.Events:WaitForChild("Place!")
local event5 = RSe.Events:WaitForChild("MOOVE")

local PosX
local PosY
local PosZ
local Rot = 0
 
local mouseTarget = Mouse.Target.Name
local Button = game.Players.LocalPlayer.PlayerGui.ScreenGui.MainGui.GSOoneblockbut
local GridSize = 2

local Fil = script.Parent.Parent.Parent.Filters
local Fact = script.Parent.Parent.Parent.Factions
local But = script.Parent.Parent.Parent.Buttus
local scroll2 = script.Parent.Parent.Parent.MainGui_12
local scroll = script.Parent.Parent.Parent.MainGui
local Keyz = script.Parent.Parent.Parent.Keyz
local Keyzz = script.Parent.Parent.Parent.Keyzz
local Keyzzz = script.Parent.Parent.Parent.Keyzzz

local CanStart = true
local IsPlacing = false
local CanPlace = false
local taable = nil


--------------------------------Functions-----------------------------------------------------------------------------




local function Snap()
 PosX = math.floor(Mouse.Hit.Position.X)
 PosY = math.floor(Mouse.Hit.Position.Y)
 PosZ = math.floor(Mouse.Hit.Position.Z)
end

local function Movement()
 if IsPlacing and CanPlace and Mouse.Target.Name == "Grid" or not Mouse.Target.Name == "Terrain" and not CanStart then
  event5:FireServer(Mouse.Target)
  event3:FireServer()
  Mouse.TargetFilter = workspace:FindFirstChild("GSOoneblock")
elseif IsPlacing and CanPlace and Mouse.Target.Name == "Terrain" --[[or Mouse.Target:IsA("Part")]] or Mouse.Target:IsA("VehicleSeat") and not CanStart then
  Snap()
  event2:FireServer(PosX,PosY,PosZ)
  event3:FireServer()
  Mouse.TargetFilter = workspace:FindFirstChild("GSOoneblock")
else
  return Mouse.Hit.p
 end
end



local function StartPlacementCrate()
 if CanStart then
  scroll:TweenPosition(UDim2.new(-0.830, 0,0.221, 0),"Out","Quad",0.4)
  scroll2:TweenPosition(UDim2.new(-0.590,0,0.539,0),"Out","Quad",0.4)
  Fact:TweenPosition(UDim2.new(-1, 0,0.221,0),"Out","Quad",0.4)
  Fil:TweenPosition(UDim2.new(-1,0,0.221, 0),"Out","Quad",0.4)
  But:TweenPosition(UDim2.new(-1,0,0.13,0),"Out","Quad",0.4)
  Keyz:TweenPosition(UDim2.new(0.426, 0,0.773, 0),"Out","Quad",0.4)
  Keyzz:TweenPosition(UDim2.new(0.277, 0,0.787, 0),"Out","Quad",0.4)
  Keyzzz:TweenPosition(UDim2.new(0.567, 0,0.787, 0),"Out","Quad",0.4)

  script.Parent:TweenPosition(UDim2.new(0.211, 0,0.180, 0),"Out","Quad",0)
  wait()
  script.Parent:Clone().Parent = Keyz
  IsPlacing = true
  CanPlace = true
  CanStart = false

  eventstart:FireServer()
  
  Mouse.Move:Connect(Movement)
  UIS.InputBegan:Connect(Movement)
  UIS.InputChanged:Connect(Movement)
  UIS.InputEnded:Connect(Movement)
  RS.Heartbeat:Connect(Movement)
  RS.Changed:Connect(Movement)
  Mouse.Button1Down:Connect(Movement)
 end
end

local function StopPlacement(input)
	if input.KeyCode == Enum.KeyCode.X and IsPlacing and CanPlace and not CanStart then
		
		Keyz:TweenPosition(UDim2.new(0.426, 0,1, 0),"Out","Quad",0.4)
		Keyzz:TweenPosition(UDim2.new(0.277, 0,1, 0),"Out","Quad",0.4)
 		Keyzzz:TweenPosition(UDim2.new(0.567, 0,1, 0),"Out","Quad",0.4)
		
		IsPlacing = false
		CanPlace = false
		CanStart = true
		
		eventdestroy:FireServer()
		wait(0.5)
		Keyz:ClearAllChildren()
	end
end


local function LowBlocks()
	if game.ReplicatedStorage.Values.GSOoneblock.Value <= 0 then
		local yeetele = Player.PlayerGui.ScreenGui.MainGui
		Keyz:TweenPosition(UDim2.new(0.426, 0,1, 0),"Out","Quad",0.4)
		Keyzz:TweenPosition(UDim2.new(0.277, 0,1, 0),"Out","Quad",0.4)
 		Keyzzz:TweenPosition(UDim2.new(0.567, 0,1, 0),"Out","Quad",0.4)

		IsPlacing = false
		CanPlace = false
		CanStart = true
		
		eventdelete:FireServer()
		yeetele.GSOoneblockbut.Visible = false
	else
		local yeetele = Player.PlayerGui.ScreenGui.MainGui
		yeetele.GSOoneblockbut.Visible = true
	end
end




local function Placement()
 if IsPlacing and CanPlace and Mouse.Target.Name == "Terrain" or not Mouse.Target.Name == "Grid" and not CanStart and game.ReplicatedStorage.Values.GSOoneblock.Value > 0 then
	Snap()
  event:FireServer()
 end
end

local function Placement2()
 if IsPlacing and CanPlace and Mouse.Target.Name == "Grid" and not CanStart then
	Snap()
	event4:FireServer(Mouse.Target)
 end
end

while IsPlacing do
	Snap()
	event2:FireServer(PosX,PosY,PosZ)
end


Mouse.Button1Down:Connect(Placement)
Mouse.Button1Down:Connect(Placement2)
Button.MouseButton1Click:Connect(StartPlacementCrate)
UIS.InputBegan:Connect(StopPlacement)
UIS.InputBegan:Connect(LowBlocks)
UIS.InputEnded:Connect(StopPlacement)
UIS.InputEnded:Connect(LowBlocks)
RS.Heartbeat:Connect(LowBlocks)

server script:

-- // Variables
local RS = game:GetService("ReplicatedStorage")
local event = RS.Events:WaitForChild("PlacementEvent")
local Model
local event2 = RS.Events:WaitForChild("Move")
local event3 = RS.Events:WaitForChild("Stop")
local eventsdelete = RS.Events:WaitForChild("NoN")
local eventdestroy = RS.Events:WaitForChild("No")
local eventstart = RS.Events:WaitForChild("Yes")
local event4 = RS.Events:WaitForChild("Place!")
local event5 = RS.Events:WaitForChild("MOOVE")
local taable = true
local taabel = {}

-- // Main


event2.OnServerEvent:Connect(function(plr,PosX,PosY,PosZ)
	Model.Parent = workspace
	Model.GSOoneblock.CanCollide = false
  	Model:SetPrimaryPartCFrame(CFrame.new(PosX,PosY+5,PosZ))
end)

event5.OnServerEvent:Connect(function(plr,mouseTarget)
	local off = CFrame.new(0,0,2)
	Model.Parent = workspace
	Model.GSOoneblock.CanCollide = false
	Model.GSOoneblock.CFrame = mouseTarget.CFrame:ToWorldSpace(off)
end)

event3.OnServerEvent:Connect(function(plr)
local collision = Model.PrimaryPart.Touched:Connect(function() end)
local collisionPoints = Model.PrimaryPart:GetTouchingParts()
for i = #collisionPoints, #collisionPoints do
	if not collisionPoints[i]:IsDescendantOf(workspace.Folder) then
		taable = true
		Model.GSOoneblock.SelectionBox.SurfaceTransparency = 1
   		Model.GSOoneblock.SelectionBox.Color3 = Color3.fromRGB(255,255,255)
	elseif collisionPoints[i]:IsDescendantOf(workspace.Folder) then
		taable = false
		Model.GSOoneblock.SelectionBox.Color3 = Color3.fromRGB(208,37,37)
		Model.GSOoneblock.SelectionBox.SurfaceTransparency = 0.8
		end
	end	
end)

event.OnServerEvent:Connect(function(plr)
if taable == true then
 local PlacedModel = Model:Clone()
 game.ReplicatedStorage.Values.GSOoneblock.Value = game.ReplicatedStorage.Values.GSOoneblock.Value -1
 PlacedModel.GSOoneblock.CanCollide = true
 PlacedModel.GSOoneblock.Anchored = false
 PlacedModel.GSOoneblock.SelectionBox.Visible = false
 PlacedModel.Parent = workspace.PlacedObjects
end
end)

eventdestroy.OnServerEvent:Connect(function(plr)
	Model:Destroy()
    wait()
	Model = nil
end)

eventsdelete.OnServerEvent:Connect(function(plr)
	Model:Destroy()
    wait()
	Model = nil
end)

eventstart.OnServerEvent:Connect(function(plr)
	Model = game:GetService("ReplicatedStorage").GSOoneblock:Clone()
	Model.GSOoneblock.CanCollide = false
end)

event4.OnServerEvent:Connect(function(plr,mouseTarget)
if taable == true then
  local PlacedModel = Model:Clone()
  game.ReplicatedStorage.Values.GSOoneblock.Value = game.ReplicatedStorage.Values.GSOoneblock.Value -1
  local weld = Instance.new("WeldConstraint")
  PlacedModel.GSOoneblock.CanCollide = true
  PlacedModel.GSOoneblock.Anchored = false
  PlacedModel.GSOoneblock.SelectionBox.Visible = false
  weld.Parent = PlacedModel
  weld.Part0 = PlacedModel.GSOoneblock
  weld.Part1 = mouseTarget
  PlacedModel.Parent = workspace.PlacedObjects
end
end)

while wait() do
	workspace:GetChildren(taabel)
	if table.find(taabel,"GSOoneblock",2) then
		Model:Destroy()
	end
end

sorry for not putting it in

ShadokuSan · March 8, 2020, 11:29pm

From what I can quickly see in the client script, you’re placing multiple connections on the mouse via the function StartPlacementCrate(). Every time that one gets called, it makes more connections to the mouse. Try removing the extra connections in there and see if it helps.

Specifically, remove lines 91-97 of the client script.

Adeenex · March 8, 2020, 11:31pm

tried just doing RS.Heartbeat:Connect(Movement) but still doesnt fix anything.

ShadokuSan · March 8, 2020, 11:43pm

Then the only other problem I can see here is that you have 2 functions that are called to clone/make a part that is connected to the mouse being clicked. They have their own checks for logic, yet both can appear to happen at the same time.

Client:

Server:

This likely just means the logic is a little off here, particularly in concern to Placement() in the client script not properly being separated. Once you put or in front of a statement and if one of the things behind it is false, it assumes everything behind it is false as well ignoring the need for IsPlacing and CanPlace to be true. You’ll want to make use of parentheses to help alleviate that issue. It would look like this:

--Let's assume VariableA and VariableB are preset variables
if IsPlacing and CanPlace and (VariableA or VariableB) then
--Code goes here
end

That will ensure that as long as one of the following is true inside the parentheses, it won’t disregard everything else before it.

Specifically redoing that function may look like this:

local function Placement()
 if IsPlacing and CanPlace and game.ReplicatedStorage.Values.GSOoneblock.Value>0 and not CanStart and (Mouse.Target.Name=="Terrain" or Mouse.Target.Name~="Grid") then
	Snap()
  event:FireServer()
 end
end

Also a little side note, =~ can be used in place of not when trying to compare to something else.

You can learn more about Conditional Structures here. It will provide examples of how to properly set things up.

IdiomicLanguage · March 8, 2020, 11:48pm

I noticed that when placing a crate that you are placing a model. I also noticed that in the video when you place the model it seems to multiply. This may be caused by multiple parts being created in the same place and then becoming unachored. This leads me to think of two possible issues:

Does the crate model have multiple parts in it? If so, unanchoring them may cause second issue of the crate placement duplicating
Is is possible that multiple copies of this local script are running? Try having the local script print out once when it starts. Printing when events happen may help as well.

Adeenex · March 8, 2020, 11:51pm

1.Yes, Capture_2020_03_08_19_49_41_63
2, No, but somethimes when game.ReplicatedStorage.Values.GSOoneblock.Value <= 0, it goes to storage, but doesnt delete it.

it is anchored when it is placing but when it is placed, its being unanchored.

Adeenex · March 8, 2020, 11:58pm

ok with that code, i get this error message:

ShadokuSan · March 8, 2020, 11:59pm

Sorry about that, made a slight mistake there. Forgot to put anything after CanStart

if IsPlacing and CanPlace and game.ReplicatedStorage.Values.GSOoneblock.Value>0 and not CanStart and (Mouse.Target.Name=="Terrain" or Mouse.Target.Name~="Grid") then

Adeenex · March 9, 2020, 12:01am

still doubling, i dont understand
sorry

ShadokuSan · March 9, 2020, 12:02am

Well in that case it is likely what IdiomicLanguage is talking about then. Just wait for their response and you should be fine.

overflowed · March 9, 2020, 12:30am

Not trying to play Kindergarten Cop but you don’t need to post the same problem twice within a three-hour time period.

IdiomicLanguage · March 9, 2020, 4:24am

Okay, I’ll put some more effort into studying this code. I’ll list my observations here and just let me know if any of them are wrong. Hopefully by the end I will find out what is wrong.

Q: what is the difference between ‘taabel’ and ‘taable’ in the server script? A: taable is actually a table and is set to an empty table. On line 91 it seems like the author assumed that the :GetChildren method populates a passed in table. This is not the case. Also the string comparison done on like 92 by find will not work for instances. Finally, starting at the second instance in the list if it were populated and filtering by names worked, it would not skip the first matching value which would be the only reason I see for attempting to start the search on the second instance. The taabel variable seems to only hold a boolean value, starts as true, and is set to true if the last part touching the PrimaryPart of Model is a descendant of workspace.Folder. This raises multiple questions:
Q: what is the significance of being a child of workspace.Folder? I have no answer for this.
Q: Why is the loop on line 34 only considering the last part touching, according to the arbitrary order given by Roblox? Functionally, this loop does not exist.
Q: What does event3 do and why does it firing control if taabel is set? A: event3 is the “Stop” remote event. It is fired client side in the Movement function and in various conditions and Movement is connected to the mouse movement, user input began / changed / ended, client heartbeats, and just in case the mouse button 1 down events. As a side note, I don’t see these connections ever being cleaned up. Anyways, these connections are made in the properly named StartPlacementCrate function.
Q: What are the conditions guarding event3 or “Stop” in the Movement function? A: it seems that the conditions are not the important factor for firing event3. It seems to be that after any movement event (event5 or event2) event3 needs to be fired. The conditions are to determine if any movement needs to take place and which type of movement.

At this point I would like to make a recommendation. When the start placement event if fired, listen to the touched and touch ended events of the primary part and use that to handle setting the selection box color / transparency. This is much more reliable then connecting an empty function to the touched event and calling a method to check all touching parts over again. Keep track of all the currently touching parts that are a descendant of workspace.Folder and remove them from the list when they are no longer touching. This will prevent adding lots of extra handlers to the touched event.

Q: What does taabel being set affect? Q: if set (not touching / colliding with objects from “workspace.Folder”) then event and event 5 will create clones of the model and seem to be finalizing placement. Otherwise placement is impossible. In other words, it seems to be a server-side collision guard. That’s good high level and secure design!
I noticed that both event2 and event5, the two movement events, set the CFrame differently. One sets Model.GSOoneblock.CFrame and the other uses Model.SetPrimaryPartCFrame. Is this intentional, and is the PrimaryPart not always GSOoneblock?
Piecing together what the rest of the events do, eventdestroy and eventsdelete have the exact same implementation. This should be noted when analysing the client code.
event and event4 seem to be the two final placement events. I just noticed that they create a clone of the model that was being dragged around and performs some basic placement tasks. This could be the source of the first issue shown in the video (two parts appearing every time the mouse is clicked) but we’ll have to visit the client code to see if the object being moved about is cleaned up with eventdestroy or eventsdelete.
event4 is a placement event that takes in the mouse target and welds the placed model .GSOoneblock to the mouse target. It is like event5 is to event2 (position vs target oriented).

On to the client code!

Q: What do the variables CanStart, IsPlacing, CanPlace, and taable control? A: CanStart is set by the StopPlacement and LowBlocks functions. It functions as a debounce on the StartPlacementCrate function.

A side note here: setting the debounce to false AFTER the wait on like 83 of the client code means that if StartPlacementCreate is called multiple times in the same frame, the debounce will not prevent them all from running at the same time! This may cause the issues seen. Always set the debounce variables ASAP after their check and never after a yielding statement like wait().

(continued) Turns out the other variables isPlacing and CanPlace are only set where CanStart is set. This means that CanStart = not IsPlacing and CanStart = not CanPlace. “One variable to rule them all!” could and probably should be used here to avoid confusion. taable along with GridSize and mouseTarget are unused variables.
Moving on to which functions are connected to which events, I noticed that the StopPlacement function is connected even before StartPlacementCrate fires. I see the conditions guarding it is the relation of the three variables mentioned in 11 and the user must press “x”. This only needs to be connected to input ended or input began, not both. However the debounce is placed before the wait here so it should have no affect.
Q: What is the LowBlocks function? Turns out it runs every heartbeat and if that wasn’t enough then it also runs when keys are pressed or released. It seems to perform the exact same operations as StopPlacement. It should be noted that it uses eventdestroy instead of eventdelete meaning that perhaps while these events have the same functionality on the backend atm that might not always be the case. One seems to be for deleting the object when there isn’t enough objects. This check should be done server side and not based on client events but server events. Optimally a new part should not be placed if there are not enough parts.

This also brings to mind another optimization. Normally placement scripts keep the part being placed locally and do not worry about moving it around on the server. When the placement is finalized with the client performing collision tests, then the client asks the server to place an object there. It does so but tells the client if the part could not be placed due to a collision error. This may require a heartbeat delay on the server side to be sure the collision tests have been run. This obsoletes event2 and event5 for movement.

Also the common code in LowBlocks and StopPlacement would be best placed in another function to reuse code and prevent the implementation from diverging in the future.

Looking at Placement and Placement2, I agree with @ShadukuSan’s comments. The common part (IsPlaceing / CanPlace / CanStart) should all be checked at the same place (optimally a single variable would be used) and if successful then the Target name would be checked. If it was Grid, then event4 would be fired, else the Terrain event.
As it turns out, since placement can continue after a copy is placed, Placement and Placement2 do not need to destroy the model being moved about. The StopPlacement / LowBlocks functions will handle cleaning it up.
Taking a closer look at the logic in Placement I see that it works… but it doesn’t work the way the author intended it. Luckily checking one of the variables is the same as checking all of them and Mouse.Target.Name == 'Terrain' is the same as not Mouse.Target.Name == 'Grid' if the only two possible names are ‘Terrain’ and ‘Grid’. The issue is that and and or operations are not associative together and have an order of operations.

Well, I’ve gone through all the code in depth now and understand it all but still can’t find the issue. The debounce bug I mentioned after 11 should be fixed. Is this all of the code present in your game? If not, strip down your game to the minimum amount required to reproduce this bug and post the place file. The issue still doesn’t seem to be in the code you have given us.

Adeenex · March 9, 2020, 7:49pm

sorry for replying so late

taabel is an empty table, and taable is a true/false value.
I dont know, and i’d like to use a different method, but im new, and dont know. This is from a sandbox game example, and i tried to import it into my game.
same as answer to question 2, it used to be 1, but changed it to that for some reason.
Yes, and as i said, im new to coding so i dont know much about remote functions, and dont know what cleaning up does for them.
oh yeah, i set it in Movement() because Movement() is fired so often.
Yes. if that second “Q” is an “A”
No, how i code is i take codes from different tutorials and make them one until i can do it by myself.
yes, i tried seeing if that effects anything.
I forgot why there is 2 “CanPlaces” there, and yes.

Adeenex · March 9, 2020, 7:51pm

The only other scripts that are in the game is the script for handelling the buttons, the one for handeling the amount of blocks text, and the one where when i step on a brick, it gives +1 block and 1 drivescript for every wheel that is placed. (haven’t added it to placement system yet)

Adeenex · March 10, 2020, 6:07pm

oh yeah now i see whats going on here, the local script IS copying… sorry for wasting your time