Hello, I’m making a WW2 story-based game. Which the whole operation is based on Operation Market Garden. In-game I made a script that stops NPC from rendering when the player is looking away.
Is there a way I could improve this script?
Sorry if I put this topic in the wrong category.
local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local Camera = game.Workspace.CurrentCamera
game:GetService("RunService").RenderStepped:Connect(function()
for i,v in pairs(workspace:GetDescendants()) do
if v.Name == "HumanoidRootPart" then
local _, CanSee = Camera:WorldToScreenPoint(v.Position)
if CanSee then
print("CanSee")
for _,d in pairs(v.Parent:GetDescendants()) do
if d:IsA("BasePart") then
d.LocalTransparencyModifier = d.Transparency
end
end
else
print("CantSee")
for _,d in pairs(v.Parent:GetDescendants()) do
if d:IsA("BasePart") then
d.LocalTransparencyModifier = 1
end
end
end
end
end
end)
A very small suggestion but you can shorten this code a bit to look better with an if-then-else expression to change the d.LocalTransparencyModifier:
local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local Camera = game.Workspace.CurrentCamera
game:GetService("RunService").RenderStepped:Connect(function()
for i,v in pairs(workspace:GetDescendants()) do
if v.Name == "HumanoidRootPart" then
local _, CanSee = Camera:WorldToScreenPoint(v.Position)
print(if CanSee then "CanSee" else "CantSee")
for _,d in pairs(v.Parent:GetDescendants()) do
if d:IsA("BasePart") then
d.LocalTransparencyModifier = if CanSee then d.Transparency else 1
end
end
end
end
end)
The very thing that’s carrying this system is also the very thing you need to get rid of to improve this ten fold and that’s getting rid of the for loop that’s iterating over the Workspace’s descendants every render step. That is so incredibly expensive it’ll cause client-side latency for a part intensive map.
Use CollectionService to mark character models instead so that you’re predictably iterating over only characters rather than doing a deep search of the Workspace for character parts every frame.
Am I in the right track?
Is this script good enough or is there something that should be fixed.
local player = game.Players.LocalPlayer
local character = player.Character or player.CharacterAdded:Wait()
local Camera = game.Workspace.CurrentCamera
local collectionservice = game:GetService("CollectionService")
local getdisappearunlook = collectionservice:GetTagged("DisappearUnlook")
local function makemodeldisappear(model,cansee)
for _,part in pairs(model:GetDescendants()) do
if part:IsA("BasePart") then
part.LocalTransparencyModifier = if cansee then part.Transparency else 1
end
end
end
game:GetService("RunService").RenderStepped:Connect(function()
for _,v in pairs(getdisappearunlook) do
local _, CanSee = Camera:WorldToScreenPoint(v.HumanoidRootPart.Position)
makemodeldisappear(v,CanSee)
end
end)
Yes, to the most reasonable extent I can think of - others may have further comments to further improve the code. Just one thing which is your if-then-else statement: I think you meant to write 0 in the first case? If LocalTransparencyModifier is ever changed and the player can see the object then it’ll just use the current value.
part.LocalTransparencyModifier = if cansee then 0 else 1
There’s just various practice problems leftover that aren’t particularly impactful but would be helpful so you can write more idiomatic code:
Use GetService for Players. Players is a service and GetService is the canonical way to fetch it. Stay consistent, given that you use GetService for RunService and CollectionService.
Use ipairs when iterating over array-like tables. GetDescendants returns such a table.
Do you by any chance know how using a ternary operator might compare to using that type of if-else block to define the value to assign to the variable?
part.LocalTransparencyModifier = canSee and part.LocalTransparencyModifier or 1
-- vs.
part.LocalTransparencyModifier = if cansee then part.LocalTransparencyModifier else 1
Lua doesn’t have ternary, this is fake ternary. Both a and b need to be true if you’re expecting the definition to be the second operand.
print(true and true or false) --> true
print(true and false or "oh") --> oh
This may produce an unexpected case where your second operand needs to be falsy because it will not pass the and and instead evaluate to whatever’s next on or. If-then-else statements allow you to predictably and clearly set what the resulting assignment should be.
those print statements are pretty expensive. it also wont hurt im sure to instead of scanning through every instance holy crap to instead use tags, i mean atleast put the npcs in a folder. that will save lots of your if checks atleast.
