A another dev said that a better structure reduces script problems and is better to read. So, how I improve the structure?
1 Like
Ok, If you wanna a example:
local module = {}
function module.attack(self: Model, plr)
-- INSTANCES
local HRP = self.PrimaryPart
local Hitbox = script.Hitbox:Clone()
local hm = self:FindFirstChild('Humanoid')
local CF = CFrame.new(0, 0.5, -2.156)
local M1 = plr.M1
local AttackOff = plr.AttackOff
local TS = game:GetService('TweenService')
local Hammer = script.Hammer:Clone()
-- ATTACK
hm.WalkSpeed = 0
hm.JumpHeight = 0
if AttackOff.Value == false then
AttackOff.Value = true
task.wait(0.1)
for _, v in pairs(Hammer:GetChildren()) do
TS:Create(v, TweenInfo.new(0.31), { ['Transparency'] = 0}):Play()
end
Hammer.Cable:WaitForChild('Arm').Part1 = self['Right Arm']
Hammer.Parent = self
local characters = {}
task.wait(0.45)
Hitbox.Parent = workspace
Hitbox.CFrame = HRP.CFrame * CF
for _, v in pairs(workspace:GetPartsInPart(Hitbox, OverlapParams.new())) do
local enemy = v.Parent
if enemy:FindFirstChild('Humanoid') and enemy ~= self and not table.find(characters, enemy) then
local eHRP = enemy.HumanoidRootPart
local Vector = Vector3.new(HRP.Position.X, eHRP.Position.Y, HRP.Position.Z)
eHRP.CFrame = CFrame.lookAt(eHRP.Position, Vector)
table.insert(characters, enemy)
TS:Create(eHRP, TweenInfo.new(0.1), { CFrame = eHRP.CFrame * CFrame.new(0, 0, 0.3)}):Play()
local hm = enemy.Humanoid
hm:TakeDamage(9)
local anim = hm.Animator:LoadAnimation(script.Hit)
anim:Play()
print('a')
end
end
Hitbox:Destroy()
task.wait(0.04)
Hammer:Destroy()
AttackOff.Value = false
hm.WalkSpeed = 16
hm.JumpHeight = 7.2
end
end
return module
1 Like
your structure looks fine but I would recommend adding more comments that tell you what each section does
1 Like
So, lets me bring other script that makes a dev of my country said about better structure
1 Like
it’s easier to read if there’s more spacing
2 Likes
It’s here the script.
local function detect(char)
local calc = (enemy.PrimaryPart.Position - char.PrimaryPart.Position).Magnitude
state = true
if calc < QuickDistance then
QuickDistance = calc
target = char
end
end
while task.wait(0.1) do
if not enemy:FindFirstChild('Humanoid') then
break
end
if enemy.PrimaryPart:FindFirstChild('Zone') then
for _, s in pairs(workspace:GetPartsInPart(enemy.PrimaryPart:WaitForChild('Zone'), OP)) do
if s.Parent:FindFirstChild("Humanoid") then
if game.Players:GetPlayerFromCharacter(s.Parent) then
if game.Players:GetPlayerFromCharacter(s.Parent).Safe.Value == false and s.Parent.Humanoid.Health > 0 then
detect(s.Parent)
end
end
end
end
if state == false then
QuickDistance = math.huge
target = nil
end
state = false
if enemy:FindFirstChild('Moving') then
if target ~= nil and enemy.PrimaryPart:FindFirstChild('Hitbox') then
enemy.Humanoid:MoveTo(target.PrimaryPart.Position)
print('s')
enemy.Moving.Value = true
enemy.Humanoid.MoveToFinished:Connect(function()
enemy.Moving.Value = false
end)
elseif (enemy.PrimaryPart.Position - pos).Magnitude > 0.05 then
enemy.Humanoid:MoveTo(pos)
enemy.Moving.Value = true
enemy.Humanoid.MoveToFinished:Connect(function()
enemy.Moving.Value = false
end)
end
end
target = nil
QuickDistance = math.huge
end
end
1 Like
perhaps make sure the animator exists, and use local hum = enemy:FindFirstChildOfClass("Humanoid") before the if block and check if the hum variable exists. then you can use that for the rest of the block.
1 Like
ok looking at your script the first thing I would suggest is to decrease the number of nested statements for readability and structure, comments and spacing would also help
1 Like
How I make this reduce? I don’t know
1 Like
you could do something like this:
for _, s in pairs(workspace:GetPartsInPart(enemy.PrimaryPart:WaitForChild('Zone'), OP)) do
if not s.Parent:FindFirstChild("Humanoid") then
continue
end
if not game.Players:GetPlayerFromCharacter(s.Parent) then
continue
end
if game.Players:GetPlayerFromCharacter(s.Parent).Safe.Value == false and s.Parent.Humanoid.Health > 0 then
detect(s.Parent)
end
end
instead of
for _, s in pairs(workspace:GetPartsInPart(enemy.PrimaryPart:WaitForChild('Zone'), OP)) do
if s.Parent:FindFirstChild("Humanoid") then
if game.Players:GetPlayerFromCharacter(s.Parent) then
if game.Players:GetPlayerFromCharacter(s.Parent).Safe.Value == false and s.Parent.Humanoid.Health > 0 then
detect(s.Parent)
end
end
end
end
2 Likes
2 Likes
I don’t understand so much the export, but I’m will use.
1 Like
Structure takes years to develop as it becomes your own style over time. Best to learn this on your own as it will cement itself in your thinking after that. For me a top down approach is the only way.
1 Like
I think the same, but there things that make better for reading, if you are working in team.
1 Like
I agree for the most part… But I also don’t like it when others can just read my scripts easily.
This is more about structure. Look up top down programming. This is time tested.
1 Like
Any seasoned scripter is going to see self: Model inside a table function and immediately be concerned.
That’s because self is special when you do:
local module = {}
function module:attack()
print(self == module)
end
module:attack()
This will print true.
I would recommend changing the variable name to something else.
2 Likes
Understandable, however it wasn’t defined using :, nor does it have self as first parameter, I think it’s fine to use self here
My mistake. self was used as 1st param, which is indeed confusing.
The continue isn’t helping in this script:
script.Parent.MouseButton1Click:Connect(function()
for _, v in pairs(workspace:GetDescendants()) do
if v:IsA('Part') or v:IsA('BasePart') then
continue
end
print(v.Material)
if v.Material ~= Enum.Material.Plastic then
v.Material = Enum.Material.SmoothPlastic
end
end
script.Parent.Text = 'POTATO MODE: ON'
end)
I’m having this error:
Material is not a valid member of Camera "Workspace.Camera"
I’m putting him here because I don’t know how I describe this, but you and me can understand the problem.