Automated swing system feels a bit.. odd

Alright, so im working on this system that automatically adds animations to play based on the amount thats in a folder. Heres my code:

			local swing_anims_ = {}

			for i, anim in pairs(viewmodel_animations:GetChildren()) do
				local name = ("swing%d"):format(i)
				if viewmodel_animations:FindFirstChild(name) then
					table.insert(swing_anims_, viewmodel_animations:WaitForChild(name)) 
				end
			end

			local values = weapon:WaitForChild("values")
			local ints = values:WaitForChild("ints")
			local max_combo = ints:WaitForChild("max_combo")

			max_combo.Value = tonumber(#swing_anims_)
			max_combo = max_combo.Value

			local chosen = swing_anims_[combo]
			local swing_anim = animator:LoadAnimation(chosen)

			swing_anim:Play()

But, I feel like its just a bit… techy i guess? it just feels like kinda… bad and unreliable. How do you think I can improve these lines of code?

Hello, I’ve tried modifying the code, however, I have no way to test it.

-- Simplified the loop by using ipairs instead of pairs
-- and using #swing_anims + 1 instead of i to generate the animation name
local swing_anims = {}
for _, anim in ipairs(viewmodel_animations:GetChildren()) do
    local name = "swing" .. #swing_anims + 1
    if viewmodel_animations:FindFirstChild(name) then
        table.insert(swing_anims, viewmodel_animations[name])
    end
end

-- Simplified the code that sets max_combo by chaining the WaitForChild calls
-- and using weapon.values.ints.max_combo instead of ints:WaitForChild("max_combo")
local max_combo = weapon.values.ints.max_combo
max_combo.Value = #swing_anims

-- Simplified the code that loads the animation by using swing_anims[combo] instead of chosen
local swing_anim = animator:LoadAnimation(swing_anims[combo])
swing_anim:Play()
1 Like

Your code looks generally fine, but there are a few areas where you can make improvements. Here are some suggestions:

  1. Avoid using “magic numbers”: Instead of using the ("%d"):format(i) format to generate animation names (“swing1”, “swing2”, etc.), you can use a more descriptive and predictable naming convention. For example, you can name your animations “swing1”, “swing2”, etc., directly in the viewmodel_animations folder. This way, you can access them using viewmodel_animations:FindFirstChild("swing" .. i).
  2. Improve error handling: Currently, you’re using WaitForChild to find animations and values. While it works in most cases, it can cause issues if the children are not found or take longer to load. Consider using FindFirstChild instead, along with conditional checks, to handle missing or delayed children gracefully.
  3. Validate animation loading: Since you’re loading animations dynamically, it’s essential to validate whether the animation is successfully loaded before attempting to play it. You can check if chosen is not nil before creating and playing the swing_anim.
  4. Add error checking for max_combo.Value: Make sure to handle cases where max_combo.Value is not a valid number or is out of range. You can use tonumber with proper error handling or a fallback value to avoid unexpected behavior.

Here’s an updated version of your code incorporating these suggestions:

local swing_anims = viewmodel_animations:GetChildren()

local max_combo = math.min(#swing_anims, 999) -- Set a maximum limit for max_combo

local ints = values:WaitForChild("ints")
local max_combo_value = ints:FindFirstChild("max_combo")

if max_combo_value then
	max_combo_value.Value = max_combo
else
	-- Handle the case where max_combo_value is not found
end

local chosen = swing_anims[combo]
if chosen then
	local swing_anim = animator:LoadAnimation(chosen)
	swing_anim:Play()
else
	-- Handle the case where chosen is not found
end

By making these improvements, your code becomes more readable, robust, and reliable. It avoids unnecessary conversions and reduces the complexity of the code.

Hello,

first of all I don’t understand why you are replying to me instead of the person that asked for help.
Second, I don’t really understand why you recently started spamming with your AI-generated solutions (if you are even human that is). With tools like GPT-4 being public on OpenAI website it’s not really needed to copy & paste things from the AI model because the original poster could’ve done it themself, instead when someone asks for code review they want a real experienced human to help them, not an AI engine, that has no way of checking if everything is correct.

1 Like

BAHHA, after some inspection of this code that he has given, it is so obviously made by ChatGPT using the way that ChatGPT talks.

Since yours is more… authentic, i will try yours instead of this AI generated response.

You can tell he just slapped this into ChatGPT because he used math.min to set a maximum limit for max combo for some reason?

Who in the world would have more than 999 animations in a single folder? Not me.

We have already established that this man is using ChatGPT, but to say why its unreliable is right there.

“Avoid using “magic numbers”: Instead of using the ("%d"):format(i) format to generate animation names (“swing1”, “swing2”, etc.)”

The point of :format() is to replace certain keywords with strings. Because of this, I didnt rename any animations. Instead, I was just labeling it as a name. In this auto generated response, I used “viewmodel_animations:FindFirstChild(name)”, however, the response just did the same exact thing but used “swing” … i, which is practically the same thing I did before.

Please, if you want to help people on Devforum, and to become a better scripter, dont get auto generated responses. They are usually unreliable, and since ChatGPT was last updated in 2019, it could give deprecated code.

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.