Help with targeting the correct part

[SodaCreamCheese (creamsoda)]

Hello, everyone I’m just starting to script, and I’m fairly sure the solution here is simple, but I can’t seem to find a solution of my own. Do tell me if the code structure is not great, I will try to improve it; as I’ve said I don’t know anything better, I just searched on the internet and picked up what works.

Goal: The ship (on the right) should fire at the existing nearest ship, but once the nearest ship is destroyed, the ship will not fire onto the nearest existing ship unless if the second ship moves closer than the first ship’s position

Issue: Ship (on the right) not firing at the second existing ship (the further one), unless second existing ship is closer than the first ship that was destroyed The issue should be around the line where assigning the NearestShip to v.

while true do
			
			-- Workspace.Ships:Childrens
			for i, v in pairs(OverallShipsFolder:GetChildren()) do
				
				-- Workspace.Ships:Childrens:Childrens
				for i, v in pairs(v:GetChildren()) do
					
					-- Checks if nearby ship doesn't belong to owner
					if v.Values["Ship.Owner"].Value ~= Ship.Parent.Name then
						local DistanceFromNearestShip = (Ship.Position - v.Position).Magnitude	
						
						if DistanceFromNearestShip <= 200 and v.Values.Stats["Ship.Integrity"].Value > 0 then
							local EnemyShipSpeed = v.Velocity.Magnitude
							
							-- If there is no nearest ship and there is a nearby ship, assign
							if not NearestShip then
								NearestShip = v
								
							-- Else if nearestship is closer to the ship then, assign
							else
								if DistanceFromNearestShip < (NearestShip.Position - Ship.Position).Magnitude then
									NearestShip = v
								end
							end
							
							-- Fire at enemy ship
							ShipState.Value = "In Combat"
							if Statistics["Attack.BulletCount"].Value ~= 0 then

								local Recoil = Vector3.new(NearestShip.Position.X  + math.random(-Statistics["Attack.Deviation"].Value, Statistics["Attack.Deviation"].Value), 1, NearestShip.Position.Z + math.random(-Statistics["Attack.Deviation"].Value, Statistics["Attack.Deviation"].Value))
								--local MovingRecoil = Vector3.new(NearestShip.Front.WorldCFrame.Position.X  + math.random(-Statistics["Attack.Deviation"].Value, Statistics["Attack.Deviation"].Value), 1, NearestShip.Front.WorldCFrame.Position.Z + math.random(-Statistics["Attack.Deviation"].Value, Statistics["Attack.Deviation"].Value))
								local NewBulletClone = Bullet:Clone()
								local NewBulletCloneVelocity = NewBulletClone.LinearVelocity

								-- Movement Speed Anticipation, if ship is moving fires in front instead of center
								if EnemyShipSpeed > 35 then
									NewBulletClone.Size = Vector3.new(Ship.Size.X / 15, 1, Ship.Size.Z / 10)
									NewBulletClone.CFrame = Ship.CFrame
									NewBulletClone.CFrame = CFrame.lookAt(NewBulletClone.Position, MovingRecoil)
									NewBulletClone.Parent = Folders.Projectiles
									NewBulletCloneVelocity.VectorVelocity = Vector3.new(200, 0, 200) * NewBulletClone.CFrame.LookVector
								else
									NewBulletClone.Size = Vector3.new(Ship.Size.X / 15, 1, Ship.Size.Z / 10)
									NewBulletClone.CFrame = Ship.CFrame
									NewBulletClone.CFrame = CFrame.lookAt(NewBulletClone.Position, Recoil)
									NewBulletClone.Parent = Folders.Projectiles
									NewBulletCloneVelocity.VectorVelocity = Vector3.new(200, 0, 200) * NewBulletClone.CFrame.LookVector
								end
								
							
								Statistics["Attack.BulletCount"].Value -= 1
								wait(0.25)

							else

								wait(Statistics["Attack.Speed"].Value)
								Statistics["Attack.BulletCount"].Value = Statistics["Attack.BurstFire"].Value

							end
							
						end
					end
				end
			end		
			
			wait()
		end	

[ThanksRoBama (Emma)]

@SodaCreamCheese

tell me if the code structure is not great

Hi, yeah it’s quite hard to follow due to the deeply nested blocks (like loops and if statements) and having all the logic in one huge block. The names you picked for the iterator variables (i and v) are also not great since they don’t say anything about what those variables actually mean. Finally to top it off you’re allowing an iterator variable from the inner loop to shadow the one from the outer loop, making it really hard to guess which one you mean.

Here’s what I mean by that last part:

	-- Workspace.Ships:Childrens
	for i, v in pairs(OverallShipsFolder:GetChildren()) do
		
		-- Workspace.Ships:Childrens:Childrens
		for i, v in pairs(v:GetChildren()) do
			
			-- Checks if nearby ship doesn't belong to owner
			if v.Values["Ship.Owner"].Value ~= Ship.Parent.Name then --Which 'v' do you mean???

In that code, here’s what I interpret your code to mean:

OverallShipsFolder is set to game.Workspace.Ships, which is a Folder in workspace that contains every ship currently in the game. A ship is probably a Model. So the outermost for loop iterates over each ship, so the “v” variable should be renamed to “ship” to indicate that it references one of the ships. The “i” variable is unused to it should be renamed to “_”, which is just another variable name but it canonically means “unused”.

The next for loop uses the same variable names, so “v” no longer means “one of the ships”. It now means “one of the children of one of the ships”, which is a fine thing to mean but you should name the variable accordingly, probably to “shipChild” if any child is relevant or “shipPart” if you only care about Parts.

The if statement then looks for Values > Ship.Owner > Value in each child of a ship, which seems like a bug. If it’s not a bug, then that’s a pretty confusing way of organizing Value objects to store information about each ship like it’s owner. It would also throw an error if any of the children of any ship doesn’t have a “Values” Folder. So hopefully you can see why it’s really confusing and why “ship” would be a better name for the outer iterator variable and “shipChild” for the inner one.

It also doesn’t help that you didn’t post the entire script when it seems like it’s pretty important to the code you did post.

If you post your updated code where you don’t use variable names like i and v I’ll take another look and see if I can actually find the bug xD Although someone else might do that regardless.

---

As for the deep nesting, you can reduce it by using continue statements, like this: Replace code like this

if v.Values["Ship.Owner"].Value ~= Ship.Parent.Name then
    local DistanceFromNearestShip = (Ship.Position - v.Position).Magnitude
    ... lots of statements

with code like this

if v.Values["Ship.Owner"].Value == Ship.Parent.Name then continue end --Notice the condition is flipped
local DistanceFromNearestShip = (Ship.Position - v.Position).Magnitude
... lots of statements

you can see how that is one level indented. You shouldn’t do it everywhere, but try it with that example and also at the if DistanceFromNearestShip <= 200 part.

[SodaCreamCheese (creamsoda)]

I’ve fixed the script, instead of destroying the ship instantly if it has 0 health, I just teleport it somewhere out of range and destroy it.