Introduce serial and parallel caster modes with SoA simulation architecture#39
Draft
weenachuangkud wants to merge 129 commits into
Draft
Introduce serial and parallel caster modes with SoA simulation architecture#39weenachuangkud wants to merge 129 commits into
weenachuangkud wants to merge 129 commits into
Conversation
- ActiveCastSerial: simplified, uses SerialSimulation - BaseCastSerial: uses SerialSimulation for all casts - SerialSimulation: single RunService, SoA, queue technique like SwiftCast
- Add ParallelSimulation: SoA pattern for each Actor - Update BaseCast: uses ParallelSimulation instead of per-cast Heartbeat - Each Actor now has one RunService handling multiple casts
- ActiveCast: Remove UpdateConnection (old per-cast heartbeat) - ActiveCastSerial: Remove metatable, use pure data structure - Now internal uses SoA, external exposes OOP API
- Add Motor6DPool for efficient Motor6D pooling - Add MovementMethod to FastCastBehavior (BulkMoveTo/Transform) - Update SerialSimulation to support Transform mode - Per-caster configuration, not global
- Update ParallelSimulation with Motor6D Transform support - Same as SerialSimulation implementation
- Added 0.1.0 changelog entry documenting new features - Created Serial benchmark (benchSerial.client.luau) - Created Parallel benchmark (benchParallel.client.luau) - Updated TODO.md with all completed items
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFastCast2 is refactored into distinct Serial and Parallel casters with SoA-driven SerialSimulation and ParallelSimulation. Trajectory storage consolidates to a single ChangesSerial and Parallel Casting Architecture Refactor with SoA Simulation
Sequence Diagram(s)sequenceDiagram
participant User
participant Bench as Benchmark Script
participant Caster as FastCast Caster
participant Sim as Serial/Parallel Simulation
User->>Bench: press E or P
Bench->>Caster: Fire AMOUNT casts
Caster->>Sim: Register casts
Sim->>Sim: process casts per frame
Bench->>Caster: Terminate casts after BENCH_TIME
Bench->>User: print FPS stats
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/FastCast2/BaseCast.luau (1)
175-191:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftCasts registered with
ParallelSimulationbut not stored inActivestable breaks cleanup and sync systems.The cast created at lines 175-191 is registered with
ParallelSimulation.Register(cast)but never added to theActivestable. However,Activesis still expected by:
HandleBulkMoveTo(line 56): IteratesActivesto batch-update cosmetic bullet positionsActiveCastCleaner.Event(line 127): ExpectsActives[activeCastID]to remove cleaned castsSyncChanges.Event(line 139): ExpectsActives[ID]to sync cast stateDestroy(line 368): IteratesActivesto terminate remaining castsAdditionally,
ParallelSimulation.Terminate()does not fire theActiveCastCleanerevent when a cast completes. The cast object is passed both cleanup and sync event references during creation, but these events will never be triggered by the parallel simulation, leaving cleanup callbacks unreachable.The same pattern exists in
Blockcast(lines 238-255) andSpherecast(lines 282-299).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FastCast2/BaseCast.luau` around lines 175 - 191, The new casts (created in the Raycast/Blockcast/Spherecast paths using ActiveCast.createCastData and then ParallelSimulation.Register(cast)) must be added into the Actives table and have their completion trigger the existing cleanup/sync events; after you call ParallelSimulation.Register(cast) add Actives[cast.ID] = cast, and ensure when the parallel simulation signals a cast completion you fire ActiveCastCleaner.Event(cast.ID) (and remove Actives[cast.ID]) and also fire SyncChanges.Event(cast.ID) (or call the same cleanup callback path used elsewhere) so ActiveCastCleaner, SyncChanges and Destroy/HandleBulkMoveTo logic see and remove these casts; update the Raycast, Blockcast, and Spherecast blocks accordingly and/or attach a termination callback from ParallelSimulation.Register to invoke ActiveCastCleaner and SyncChanges for the cast.src/FastCast2/ActiveCast.luau (1)
367-380:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFinish the
Trajectoryrename insideSimulateCast().These lines switch to
local trajectory = cast.StateInfo.Trajectory, but the function still readslatestTrajectorylater on. That variable is undefined here, so the first simulation step will nil-index onStartTime/Acceleration.Suggested fix
local trajectory = cast.StateInfo.Trajectory @@ - totalDelta = cast.StateInfo.TotalRuntime - latestTrajectory.StartTime + totalDelta = cast.StateInfo.TotalRuntime - trajectory.StartTime @@ - if typeof(latestTrajectory.Acceleration) ~= "Vector3" then - latestTrajectory.Acceleration = Vector3.new() + if typeof(trajectory.Acceleration) ~= "Vector3" then + trajectory.Acceleration = Vector3.new() endAlso applies to: 416-418
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FastCast2/ActiveCast.luau` around lines 367 - 380, In SimulateCast(), you started using local trajectory = cast.StateInfo.Trajectory but left uses of latestTrajectory later; replace every use of latestTrajectory with trajectory (e.g., when computing totalDelta and accessing StartTime/Acceleration) so lines that compute totalDelta, and any subsequent references (also around the block at the other occurrence ~416-418) use trajectory.StartTime, trajectory.Acceleration, etc.; verify GetPositionAtTime/GetVelocityAtTime calls and TotalRuntime updates all reference trajectory instead of latestTrajectory.README.md (1)
200-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse
Hit, notRayHit, in the events module example.The runtime lookup uses
FastCastEvents.Hit;module.RayHitwill never be invoked.Suggested fix
-module.RayHit = function() +module.Hit = function() print("Hit!") end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 200 - 202, The example registers the wrong event name; replace the handler assignment of module.RayHit with the runtime-expected FastCastEvents.Hit by defining module.Hit (or assigning to module[FastCastEvents.Hit]) so the runtime lookup calls it—update the example to use module.Hit instead of module.RayHit and ensure the string/constant FastCastEvents.Hit is referenced consistently with the rest of the module.src/FastCast2/init.luau (2)
128-145:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftUnify the
StateInfo.Trajectoryshape before using it here.This file currently treats
Trajectoryas both an array (trajectories[index],table.insert(...)) and a single segment (trajectory.StartTime). It also writes throughlastTrajectory, which is never defined in this scope. The first transform update on this path will hit a nil access or mutate the wrong structure.Also applies to: 154-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FastCast2/init.luau` around lines 128 - 145, GetTrajectoryInfo and GetLatestTrajectoryEndInfo assume StateInfo.Trajectory is an array but elsewhere it may be a single segment and code writes through lastTrajectory (undefined here), causing nil/mutation bugs; fix by normalizing StateInfo.Trajectory to a consistent array shape before use (e.g., if StateInfo.Trajectory is a map with StartTime, wrap it in a one-element array) and update any writers to always push/replace elements on that array, and remove or properly define lastTrajectory (use the local variable trajectory or StateInfo.Trajectory[#] instead) so GetTrajectoryInfo(GetLatestTrajectoryEndInfo) never indexes a non-array or mutates the wrong structure.
645-651:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn after warning when the parallel caster is not initialized.
This guard only logs, then immediately calls
self.Dispatcher:DispatchAll(...)anyway. Ifself.Dispatcheris nil,SetBulkMoveEnabled()still crashes on the next line.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FastCast2/init.luau` around lines 645 - 651, In SetBulkMoveEnabled, the guard only warns when the caster isn't initialized but proceeds to call self.Dispatcher:DispatchAll and set self.BulkMoveEnabled, causing a crash if self.Dispatcher is nil; update the function (FastCastParallel:SetBulkMoveEnabled) to return immediately after the warn (or otherwise check for self.Dispatcher) so DispatchAll and setting BulkMoveEnabled only run when self.AlreadyInit and self.Dispatcher are valid.
🧹 Nitpick comments (2)
src/FastCast2/Motor6DPool.luau (1)
46-53: 💤 Low valueConsider simplifying
Get()to avoid recursion.The recursive call after
GrowPoolworks correctly but could be simplified with a direct return.Simplified version
local function Get(): Motor6D - if `#FreeMotor6Ds` > 0 then - return table.remove(FreeMotor6Ds) :: Motor6D - else + if `#FreeMotor6Ds` == 0 then GrowPool(PoolSize * GROWTH_RATE) - return Get() end + return table.remove(FreeMotor6Ds) :: Motor6D end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/FastCast2/Motor6DPool.luau` around lines 46 - 53, The Get function currently recurses after calling GrowPool; replace that recursion with direct logic: call GrowPool(PoolSize * GROWTH_RATE) when FreeMotor6Ds is empty and then immediately remove and return a Motor6D from FreeMotor6Ds. Update the Get function to check FreeMotor6Ds, call GrowPool if empty, then perform a single table.remove(FreeMotor6Ds) :: Motor6D return; keep references to FreeMotor6Ds, GrowPool, PoolSize and GROWTH_RATE intact.Benchmarks/benchSerial.client.luau (1)
55-70: 💤 Low valueConsider adding
ProjectileContainerto filter exclusions.The
FilterDescendantsInstancesis empty, butcastBehavior.AutoIgnoreContainer = trueshould handle this. However, explicitly adding the container ensures it's excluded even before AutoIgnore processes it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Benchmarks/benchSerial.client.luau` around lines 55 - 70, The RaycastParams FilterDescendantsInstances is left empty which can let the ProjectileContainer be hit before AutoIgnoreContainer runs; update CastParams.FilterDescendantsInstances to include the ProjectileContainer instance so the container is explicitly excluded from raycasts (adjust the CastParams used by castBehavior), i.e. add ProjectileContainer to CastParams.FilterDescendantsInstances so CastParams (used by castBehavior.RaycastParams) will proactively ignore that container.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Benchmarks/benchSerial.client.luau`:
- Around line 106-121: The current random direction uses integer
math.random(-1,1) which can produce a zero vector; update the Caster:RaycastFire
call to build float random components instead (e.g. x = (math.random() * 2 - 1)
* 5000) and normalize the Vector3 before passing it, and if the normalized
vector has zero magnitude replace it with a safe fallback direction (e.g.
Vector3.new(0,0,1)); apply these changes where Caster:RaycastFire is invoked so
the direction passed to RaycastFire is never a zero vector and maintains
expected length handling with the 35 parameter and castBehavior.
In `@README.md`:
- Around line 96-100: The README example uses FastCastEnums but never defines or
requires it, so copy-pasting fails; update the example to first require or
define FastCastEnums (the same module providing enums used by FastCast2) before
creating the behavior—e.g., add a line showing how to obtain FastCastEnums (the
module that exposes HighFidelityBehavior) and then keep the existing code that
calls FastCast2.newBehavior() and sets behavior.HighFidelityBehavior =
FastCastEnums.HighFidelityBehavior.Default so the snippet runs as-is.
In `@src/FastCast2/ActiveCast.luau`:
- Around line 854-857: High-fidelity branch still reads from latestTrajectory
instead of the new local trajectory, causing the crash in
HighFidelityBehavior.Always; update the Always branch to use the local variable
trajectory (from cast.StateInfo.Trajectory, where you already default
trajectory.Acceleration) when pulling origin/startTime/velocity/acceleration
(replace references to latestTrajectory in the HighFidelityBehavior.Always code
path with trajectory) so the branch uses the validated trajectory object.
In `@src/FastCast2/ActiveCastSerial.luau`:
- Around line 148-156: The RayInfo table in ActiveCastSerial.luau doesn't copy
shape fields, so SerialSimulation.Register (which reads cast.RayInfo.Size and
cast.RayInfo.Radius for Blockcast/Spherecast) sees nil; update the constructor
that builds RayInfo to include Size = castData.Size and Radius = castData.Radius
(or the appropriate source names on castData) so Blockcast and Spherecast have
their shape parameters when Type is set via CastVariantTypes[castData.CastType];
ensure the names match what SerialSimulation.Register expects.
In `@src/FastCast2/BaseCastSerial.luau`:
- Around line 32-45: The module-level mutable state (Output, ParentCaster,
ObjectCache, BulkMoveToConnection, NextProjectileID) must be made instance-local
so multiple BaseCastSerial instances do not share or clobber each other; modify
BaseCastSerial.Init to set self.Output, self.ParentCaster, self.ObjectCache,
self.BulkMoveToConnection and self.NextProjectileID (and initialize them
appropriately) instead of writing the module-scope variables, then update all
functions in BaseCastSerial that currently reference Output, ParentCaster,
ObjectCache, BulkMoveToConnection or NextProjectileID to use self.Output,
self.ParentCaster, self.ObjectCache, self.BulkMoveToConnection and
self.NextProjectileID so every caster keeps its own event sink/cache/parent and
ID counter.
In `@src/FastCast2/init.luau`:
- Around line 919-935: FastCastSerial:TerminateCast currently only disconnects
local state and destroys cosmetic objects but never delegates termination to the
base serial, leaving the cast registered; update FastCastSerial:TerminateCast to
call self.BaseCast:TerminateCast(cast, castTerminatingFunction) (instead of
directly invoking castTerminatingFunction) after performing the local cleanup
(disconnecting UpdateConnection and destroying CosmeticBulletObject) and before
firing self.Output:Fire("CastTerminating", cast) so the cast is properly
deregistered from SerialSimulation/BaseCast.
- Around line 815-818: GetVelocityCast in FastCastSerial currently returns the
trajectory's InitialVelocity, which ignores acceleration and elapsed time;
update GetVelocityCast(cast: vaildcast) to compute velocity at runtime by using
cast.StateInfo.Trajectory and the elapsed time (use TotalRuntime - StartTime
from the same cast.StateInfo like the position helper does), then evaluate the
trajectory's velocity at that elapsed time and return it instead of
InitialVelocity so accelerating serial casts report the current velocity.
- Around line 847-903: The setters/adders (SetVelocityCast, SetAccelerationCast,
SetPositionCast, AddPositionCast, AddVelocityCast, AddAccelerationCast) mutate
cast.StateInfo.Trajectory in-place without advancing StartTime to the cast's
TotalRuntime, causing retroactive path changes; before changing
Origin/InitialVelocity/Acceleration you must "rebase" the serial trajectory by
integrating the current Trajectory forward to cast.StateInfo.TotalRuntime
(compute the new Origin at that time), set Trajectory.StartTime =
cast.StateInfo.TotalRuntime, and replace
Trajectory.Origin/InitialVelocity/Acceleration with values relative to that new
StartTime (or add deltas for the Add* methods) so subsequent solves treat the
change as occurring only from the current runtime onward. Ensure this rebase
logic is invoked from each of the listed methods (or factor into a helper like
RebaseSerialTrajectory(cast) and call it first).
- Around line 1034-1044: The constructors FastCast.new and FastCast.newParallel
build caster tables (e.g., local fs = { ... } and local fs = { ... }) and set
their metatables (setmetatable(fs, FastCastSerial) / setmetatable(fs,
FastCastParallel)), but currently don't return them; update both functions to
return the constructed fs at the end so callers receive the caster instance
(ensure return fs is added after setmetatable in both FastCast.new and
FastCast.newParallel).
- Around line 721-733: The serial BindableEvent created as BindableOutput is not
wired to the public signals after calling BaseCastSerial.Init, so events like
"CastFire" and "CastTerminating" emitted by self.BaseCast never reach consumers;
add a listener on BindableOutput.Event (after self.BaseCast =
BaseCastSerial.Init(...)) that inspects the incoming event name and forwards it
to the corresponding public signal on this instance (for example, map "CastFire"
-> self.CastFire:Fire(...), "CastTerminating" -> self.CastTerminating:Fire(...),
and forward any other serial event names similarly) so that
FastCastSerial:Init/instance exposes the same callbacks as BaseCastSerial.
In `@src/FastCast2/ParallelSimulation.luau`:
- Around line 263-270: The call to CastHandler is passing a table named variant
for all cast types but Blockcast expects a Vector3 and Spherecast expects a
number; change how variant is constructed so that for EnumCastTypes.Blockcast
you set variant to castSize[i] (a Vector3) and for EnumCastTypes.Spherecast set
variant to castRadius[i] (a number), leaving the existing table form only for
other cast types if needed, then call CastHandler(castWorldRoot[i],
lastPosition, rayDir, castRaycastParams[i], variant).
- Around line 313-316: The loop that calls ParallelSimulation.Terminate over
destroyedIds must iterate in descending numeric order because Unregister() uses
swap-delete and removing lower indices first can change positions of higher
indices; update the code that processes destroyedIds (the block calling
ParallelSimulation.Terminate) to sort or iterate destroyedIds from highest to
lowest (e.g., iterate numeric indices in reverse) so Terminate is invoked on IDs
in descending order to avoid invalidating queued ids after Unregister().
- Around line 161-221: The removal logic swaps the tail (lastId) into slot id
but then disconnects and clears fields from id (corrupting the moved cast); fix
by treating lastId as the source: when id ~= lastId assign casts[id] =
casts[lastId] (in addition to the per-field arrays) and update casts[lastId].ID
= id, then call Motor6DPool.Disconnect(castMotor6D[lastId]) and nil out all
cast*[...] entries using lastId (not id) before decrementing castCount; ensure
castCFrame and castMotor6D clears reference lastId so the moved cast in slot id
remains intact.
- Around line 321-324: ParallelSimulation.Start currently connects RS.PreRender
(client-only) which will error on servers; modify ParallelSimulation.Start to
first check the RunService client/server state (use RS:IsClient() or similar)
and only call RS.PreRender:ConnectParallel(UpdateCasts) when running on the
client, or alternatively use RS.Heartbeat for server-side behavior; ensure you
still set and guard ParallelSimulation.StepConnection and early-return if
already connected. This change keeps ParallelSimulation.Start, UpdateCasts, and
ParallelSimulation.StepConnection as the anchors for the fix.
In `@src/FastCast2/SerialSimulation.luau`:
- Around line 317-320: The loop that calls SerialSimulation.Terminate for each
id in destroyedIds must iterate destroyedIds in descending index order to avoid
invalidation caused by swap-delete compaction; change the iteration over
destroyedIds so you process highest indices first (e.g., iterate from
`#destroyedIds` down to 1) and call SerialSimulation.Terminate(id) in that
descending order to ensure removals do not shift and invalidate later ids.
- Around line 263-274: The code always passes the variant table to CastHandler,
but Blockcast and Spherecast handlers expect a raw Vector3/number; update the
call so it passes variant.Size for EnumCastTypes.Blockcast and variant.Radius
for EnumCastTypes.Spherecast (use castSize[i] and castRadius[i] or the variant
fields you already set) instead of the whole variant table when castType is
Blockcast or Spherecast; keep passing variant only for other cast types. Ensure
this change is applied at the CastHandler(...) invocation so the correct shape
argument is forwarded to block/sphere handlers.
- Around line 164-225: The code in Unregister swaps the tail row into id but
then disconnects and clears entries at index id (which now holds the moved
cast), causing the moved cast to be corrupted; change the cleanup to operate on
lastId instead: perform Motor6DPool.Disconnect(castMotor6D[lastId]) and set
castIDs[lastId], castOrigin[lastId], ..., castCFrame[lastId],
castMotor6D[lastId], casts[lastId] = nil, then set castCount = lastId - 1. Keep
the earlier copy/ID update that moves lastId into id, but ensure all nil
assignments and the disconnect target lastId (not id) so the moved cast at id
remains intact.
---
Outside diff comments:
In `@README.md`:
- Around line 200-202: The example registers the wrong event name; replace the
handler assignment of module.RayHit with the runtime-expected FastCastEvents.Hit
by defining module.Hit (or assigning to module[FastCastEvents.Hit]) so the
runtime lookup calls it—update the example to use module.Hit instead of
module.RayHit and ensure the string/constant FastCastEvents.Hit is referenced
consistently with the rest of the module.
In `@src/FastCast2/ActiveCast.luau`:
- Around line 367-380: In SimulateCast(), you started using local trajectory =
cast.StateInfo.Trajectory but left uses of latestTrajectory later; replace every
use of latestTrajectory with trajectory (e.g., when computing totalDelta and
accessing StartTime/Acceleration) so lines that compute totalDelta, and any
subsequent references (also around the block at the other occurrence ~416-418)
use trajectory.StartTime, trajectory.Acceleration, etc.; verify
GetPositionAtTime/GetVelocityAtTime calls and TotalRuntime updates all reference
trajectory instead of latestTrajectory.
In `@src/FastCast2/BaseCast.luau`:
- Around line 175-191: The new casts (created in the
Raycast/Blockcast/Spherecast paths using ActiveCast.createCastData and then
ParallelSimulation.Register(cast)) must be added into the Actives table and have
their completion trigger the existing cleanup/sync events; after you call
ParallelSimulation.Register(cast) add Actives[cast.ID] = cast, and ensure when
the parallel simulation signals a cast completion you fire
ActiveCastCleaner.Event(cast.ID) (and remove Actives[cast.ID]) and also fire
SyncChanges.Event(cast.ID) (or call the same cleanup callback path used
elsewhere) so ActiveCastCleaner, SyncChanges and Destroy/HandleBulkMoveTo logic
see and remove these casts; update the Raycast, Blockcast, and Spherecast blocks
accordingly and/or attach a termination callback from
ParallelSimulation.Register to invoke ActiveCastCleaner and SyncChanges for the
cast.
In `@src/FastCast2/init.luau`:
- Around line 128-145: GetTrajectoryInfo and GetLatestTrajectoryEndInfo assume
StateInfo.Trajectory is an array but elsewhere it may be a single segment and
code writes through lastTrajectory (undefined here), causing nil/mutation bugs;
fix by normalizing StateInfo.Trajectory to a consistent array shape before use
(e.g., if StateInfo.Trajectory is a map with StartTime, wrap it in a one-element
array) and update any writers to always push/replace elements on that array, and
remove or properly define lastTrajectory (use the local variable trajectory or
StateInfo.Trajectory[#] instead) so
GetTrajectoryInfo(GetLatestTrajectoryEndInfo) never indexes a non-array or
mutates the wrong structure.
- Around line 645-651: In SetBulkMoveEnabled, the guard only warns when the
caster isn't initialized but proceeds to call self.Dispatcher:DispatchAll and
set self.BulkMoveEnabled, causing a crash if self.Dispatcher is nil; update the
function (FastCastParallel:SetBulkMoveEnabled) to return immediately after the
warn (or otherwise check for self.Dispatcher) so DispatchAll and setting
BulkMoveEnabled only run when self.AlreadyInit and self.Dispatcher are valid.
---
Nitpick comments:
In `@Benchmarks/benchSerial.client.luau`:
- Around line 55-70: The RaycastParams FilterDescendantsInstances is left empty
which can let the ProjectileContainer be hit before AutoIgnoreContainer runs;
update CastParams.FilterDescendantsInstances to include the ProjectileContainer
instance so the container is explicitly excluded from raycasts (adjust the
CastParams used by castBehavior), i.e. add ProjectileContainer to
CastParams.FilterDescendantsInstances so CastParams (used by
castBehavior.RaycastParams) will proactively ignore that container.
In `@src/FastCast2/Motor6DPool.luau`:
- Around line 46-53: The Get function currently recurses after calling GrowPool;
replace that recursion with direct logic: call GrowPool(PoolSize * GROWTH_RATE)
when FreeMotor6Ds is empty and then immediately remove and return a Motor6D from
FreeMotor6Ds. Update the Get function to check FreeMotor6Ds, call GrowPool if
empty, then perform a single table.remove(FreeMotor6Ds) :: Motor6D return; keep
references to FreeMotor6Ds, GrowPool, PoolSize and GROWTH_RATE intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a57e535-492e-4838-826f-86da925a99ab
📒 Files selected for processing (18)
Benchmarks/benchParallel.client.luauBenchmarks/benchSerial.client.luauREADME.mdTODO.mddocs/api-reference.mddocs/changelog.mdsrc/FastCast2/ActiveCast.luausrc/FastCast2/ActiveCastSerial.luausrc/FastCast2/BaseCast.luausrc/FastCast2/BaseCastSerial.luausrc/FastCast2/Configs.luausrc/FastCast2/DefaultConfigs.luausrc/FastCast2/FastCastEnums.luausrc/FastCast2/Motor6DPool.luausrc/FastCast2/ParallelSimulation.luausrc/FastCast2/SerialSimulation.luausrc/FastCast2/TypeDefinitions.luausrc/FastCast2/init.luau
weenachuangkud
added
documentation
…astParallel - Add SoA pattern with full cast data arrays (Paused, TotalRunTime, Trajectory, etc.) - Implement Register/Unregister/SetMovementMode/QueueEvent methods - Add GetSoA() to expose arrays for user-implemented UpdateCasts - Add GetBaseCastRef() to access ObjectCache/Motor6DCache methods - Fix BulkMoveToConnection undefined reference in Destroy() - Add Motor6DCache require and instance management - Add BindMotor6DCache, _GetMotor6D, _ReturnMotor6D methods - Update SetMovementMode to delegate to ParallelSimulation - Add FireQueuedEvents for event handling after sync - Init now starts ParallelSimulation and connects RunService
…on SoA - Update FireQueuedEvents to use castID + args (varargs) from ParallelSimulation - Update HandleBulkMoveTo/HandleMotor6D to get data from GetSoA() - Add Start()/Stop() methods to ParallelSimulation for RunService connection - Add ParallelSimulation.Connection for connection management
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores