Feedback on Refactored Movement and Camera Systems

Help and Feedback Code Review
#1

I’ve recently undertaken a refactor of several key systems in my Roblox horror game, organizing everything into separate modules and folders to improve structure, readability, and maintainability.

Here’s the previous structure:

1
11366×514 110 KB

Here’s the current structure:

2
21366×513 102 KB

  • PlayerDataModule: Stores and tracks various player states such as input presses and movement modes.
    • FirstPerson: Includes scripts for CameraPos, CameraSway, HeadMovement, FOV, and Transparency adjustments.
    • Inputs: Dedicated to tracking all player input within one script.
    • Movement: Divided into individual scripts for Walking, Sprinting, Jumping, Crouching, and Prone.

I’d appreciate any thoughts or suggestions on how I can make further improvements!

  • The refactor looks great!
  • It needs more work.
  • Too Complex

0 voters

#2

Hi! Your refactor looks great! I love how you separated concerns and grouped every aspect into a folder, keeping it clean and understandable for the future. Looks great!

Here’s a suggestion for the future, I think you could implement/use the SSA (Single Script Architechture into your game, in other words Modular Programming, here’s a solution on this other thread that explains why implementing such architechture is good for you.

Overall, your work is amazing and even your code looks clean!

1 Like
#3

You should define variables outside the loop, and assign them a value inside the loop for each iteration.

-- allocate a storage in memory and let the compiler know what kind of data should the object handle
local swayAmount: number
local MouseDelta: Vector2

-- consider changing the way you approach getting the most up to date value of IsSprinting
local isSprinting: boolean

defining a variable inside a loop means you are allocating new storage in memory for each iteration or frame of the loop. It’s not harmful in this particular situation but it’s always more efficient to reuse the same storage.

RunService:BindToRenderStep(..., function()
      MouseDelta = UserInputService:GetMouseDelta()
      swayAmount = IsSprinting and 12 or 8
end)

keep in mind that each new allocated storage that you lose the reference of, are scheduled to be garbage collected, so it can take some time before all the memory that accumulated to be freed by the garbage collector.

1 Like