Skip to content

BF-23 | ZRobisz guwno#28

Open
kardavx wants to merge 7 commits intomasterfrom
im-going-insane
Open

BF-23 | ZRobisz guwno#28
kardavx wants to merge 7 commits intomasterfrom
im-going-insane

Conversation

@kardavx
Copy link
Copy Markdown
Owner

@kardavx kardavx commented Oct 30, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Added a priority-based animation and pose system for improved item handling and character animations.
    • Introduced sensitivity modifiers for refined camera control during freelook and movement states.
    • Added debug visualization system for enhanced development and troubleshooting.
  • Improvements

    • Enhanced freelook responsiveness with adjusted sensitivity and tighter vertical constraints.
    • Refined camera handling and item initialization timing for smoother gameplay transitions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 30, 2025

Walkthrough

Introduces a new Animation controller orchestrating dual-context animation playback (viewmodel and character), extends camera controls with sensitivity modifiers, adds a Shobnode debug UI system, refactors item animations to use centralized animation handling, removes the legacy viewmodel controller, and updates animation configurations across weapon items to use priority-based selection instead of weights.

Changes

Cohort / File(s) Summary
Animation System
src/client/controllers/animation.ts, src/client/functions/items/create_viewmodel.ts
New Animation controller manages synchronized dual-context animation playback with typeLookup tracking, dynamic loading from item configs, and per-frame updates. Integration hook added to viewmodel creation flow.
Camera & Input Control
src/client/controllers/camera.ts, src/client/controllers/freelook.ts
Camera gains new SensitivityModifier system with per-name modifiers and global multiplier computation; Freelook y-axis clamping tightened (1 → 0.4) and sensitivity increased (300 → 270).
Movement & Character Handling
src/client/controllers/movement.ts, src/client/controllers/items.ts, src/client/controllers/movement_sounds.ts
Movement refactored to use cameraController dependency and implements OnFallChanged/OnRunningChanged with sensitivity modifiers; Items adds started flag for conditional slot selection; MovementSounds implements OnRender for frame updates.
Base Item Refactoring
src/client/items/base_item.ts
Replaced legacy Animator-based animations with centralized Animation controller API, refactored camera animation modifiers, added torso orientation reconstruction for viewmodel transforms.
Removed Component
src/client/controllers/viewmodel.ts
Deleted legacy Viewmodel controller that cloned and placed viewmodels in temporary folder during initialization.
Debug UI System (Shobnode)
src/client/shobnode/index.tsx, src/client/shobnode/ui/ShobnodeNode.tsx, src/client/shobnode/ui/ShobnodeOutput.tsx, src/client/shobnode/ui/ShobnodeTable.tsx
New Roact-based debug UI subsystem with dynamic node positioning via camera projection, variable display, logging facility, and collapsible tree view rendering.
Type & Configuration Updates
src/client/types/items.ts, src/shared/configurations/items/SR-16/animations.ts, src/shared/configurations/items/SR-16/properties.ts, src/shared/configurations/items/ak_47/animations.ts, src/shared/configurations/items/default/animations.ts, src/shared/configurations/items/tokarev_tt_33/animations.ts
Viewmodel types extended with CameraBone Motor6D and arm parts; animation configs refactored from weight-based to priority-based selection; idle animations converted to Pose type; aimOffset property added.
Utility Functions
src/shared/getObjectSize.ts
New utility function to count object properties via pairs iteration.

Sequence Diagram(s)

sequenceDiagram
    participant Item as Item (base_item)
    participant AnimCtrl as Animation Controller
    participant CanimVW as Canim (Viewmodel)
    participant CanimChr as Canim (Character)
    participant ItemCfg as Item Config

    rect rgb(200, 220, 255)
    Note over Item,ItemCfg: Initialization (onInit)
    Item->>AnimCtrl: onInit()
    AnimCtrl->>ItemCfg: Iterate configs
    ItemCfg-->>AnimCtrl: Animation/Pose metadata
    AnimCtrl->>AnimCtrl: Load animations async<br/>(globalChecksum)
    AnimCtrl->>AnimCtrl: Store tracks & typeLookup
    end

    rect rgb(200, 220, 255)
    Note over Item,CanimChr: Runtime - Play Animation
    Item->>AnimCtrl: playAnimation("reload")
    AnimCtrl->>AnimCtrl: Resolve type via typeLookup
    AnimCtrl->>CanimVW: play_animation or play_pose
    AnimCtrl->>CanimChr: play_animation or play_pose
    CanimVW-->>AnimCtrl: Track
    CanimChr-->>AnimCtrl: Track
    AnimCtrl-->>Item: CanimTracks
    end

    rect rgb(200, 220, 255)
    Note over Item,CanimChr: Per-Frame Update (onRender)
    Item->>AnimCtrl: onRender(dt)
    AnimCtrl->>CanimVW: Update state
    AnimCtrl->>CanimChr: Update state
    AnimCtrl->>AnimCtrl: Render Shobnode debug display
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Animation system integration: New Animation controller introduces complex async loading, dual-context management, and track lifecycle. Cross-dependencies with viewmodel creation and base_item refactoring require careful verification.
  • base_item.ts refactoring: Substantial rewrite replacing legacy Animator patterns; changes to camera modifier lifecycle, torso transform calculations, and animation state transitions need thorough testing of equip, reload, and idle flows.
  • Camera sensitivity modifiers: New per-name modifier system with global multiplier; verify dampening, destruction semantics, and interaction with existing FOV modifier logic.
  • Shobnode UI system: Large new module with React-like rendering, world-to-screen projection, and logging infrastructure; requires validation of binding updates and lifecycle management.
  • Configuration migrations: Multiple animation config files transitioning from weights to priority system; verify backward compatibility and prioritization logic.

Poem

🐰 Hops through frames with anims aligned,
Dual dancers sync'd in viewmodel's mind,
Camera bends to the modifier's tune,
While Shobnode glows—a debug commune,
Priorities set, old weights leave behind! 🎬✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The pull request title is "BF-23 | ZRobisz guwno". While "BF-23" appears to be a ticket reference, the descriptive portion "ZRobisz guwno" is vague and does not clearly communicate what the changeset implements. This changeset is substantial and includes major additions (new Animation controller, Shobnode UI system) and significant refactoring across multiple controllers, yet the title provides no meaningful indication of these changes. The descriptive text appears to be placeholder, gibberish, or non-descriptive terminology that fails to convey what a teammate scanning git history would learn about the purpose and scope of the changes. The title should be revised to clearly describe the main objectives of this substantial changeset. Consider a title like "BF-23 | Introduce Animation controller and Shobnode UI system with controller refactoring" or "BF-23 | Add Animation system and UI framework with movement/camera controller updates" to meaningfully communicate the primary changes and help team members understand the PR's purpose without needing to review the full changeset.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch im-going-insane

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/client/controllers/movement_sounds.ts (1)

53-124: Stop currentlyPlaying from growing without bound.
Every new sound appends a label to currentlyPlaying, but nothing ever removes the entry, so the array grows forever and the Shobnode panel fills with stale items. Prune the entry when the sound finishes (or otherwise) before calling display_node.

Apply this diff:

-        this.currentlyPlaying.push(`${materialName}/${name}${soundIndex} ${volume}`);
+        const entry = `${materialName}/${name}${soundIndex} ${volume}`;
+        this.currentlyPlaying.push(entry);
         const sound = this.createSound(soundId, volume);
         sound.Play();
         task.delay(sound.TimeLength, () => {
             if (name === "Turn") this.turnSoundCooldown = false;
+            const idx = this.currentlyPlaying.indexOf(entry);
+            if (idx !== -1) this.currentlyPlaying.splice(idx, 1);
             if (sound && sound.Parent) sound.Destroy();
         });
src/client/items/base_item.ts (1)

253-261: Reset the camera modifier on destroy.
Leaving the camera modifier at the last viewmodel transform keeps the camera offset after the item is unequipped because nothing else clears it. Reset it before teardown.

Apply this diff:

-		this.animation.stopAnimation(`${this.itemName}/idle`);
-		this.unbindActions();
+		this.animation.stopAnimation(`${this.itemName}/idle`);
+		this.cameraAnimationModifier.setOffset(new CFrame());
+		this.unbindActions();
🧹 Nitpick comments (3)
src/shared/getObjectSize.ts (1)

3-5: Consider simplifying since key and value are unused.

Since the actual key and value aren't used in the loop body (only the iteration count matters), you could simplify this slightly.

Apply this diff for a cleaner implementation:

-	for (const [value, key] of pairs(object)) {
+	for (const [_key, _value] of pairs(object)) {
 		size += 1;
 	}

Or even simpler, if your linter allows it:

-	for (const [value, key] of pairs(object)) {
+	for (const _ of pairs(object)) {
 		size += 1;
 	}
src/client/shobnode/ui/ShobnodeOutput.tsx (1)

10-12: Clarify position binding usage.

The binding is created but never updated (always remains new UDim2()), and all labels use binding[0] for their position. Since the uilistlayout on Line 40 handles positioning automatically, this static binding appears unnecessary. Consider either removing the position prop from labels or clarifying if dynamic positioning is planned for future iterations.

If the binding is truly unused, apply this diff:

-let binding = Roact.createBinding(new UDim2());
 const label = (text: string, textcolor?: Color3) => {
-	return <ShobnodeNode size={8} position={binding[0]} data={[text]} color={textcolor || new Color3(1, 1, 1)} />;
+	return <ShobnodeNode size={8} data={[text]} color={textcolor || new Color3(1, 1, 1)} />;
 };
src/client/shobnode/ui/ShobnodeNode.tsx (1)

8-11: Wire up anchor_right or drop the prop.
anchor_right is part of the public props but the component never reads it, so callers get no effect when they set it. Either implement the right-anchoring behavior or remove the prop to keep the API honest.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe86285 and ae11b0c.

📒 Files selected for processing (20)
  • src/client/controllers/animation.ts (1 hunks)
  • src/client/controllers/camera.ts (3 hunks)
  • src/client/controllers/freelook.ts (2 hunks)
  • src/client/controllers/items.ts (3 hunks)
  • src/client/controllers/movement.ts (6 hunks)
  • src/client/controllers/movement_sounds.ts (6 hunks)
  • src/client/controllers/viewmodel.ts (0 hunks)
  • src/client/functions/items/create_viewmodel.ts (3 hunks)
  • src/client/items/base_item.ts (10 hunks)
  • src/client/shobnode/index.tsx (1 hunks)
  • src/client/shobnode/ui/ShobnodeNode.tsx (1 hunks)
  • src/client/shobnode/ui/ShobnodeOutput.tsx (1 hunks)
  • src/client/shobnode/ui/ShobnodeTable.tsx (1 hunks)
  • src/client/types/items.ts (1 hunks)
  • src/shared/configurations/items/SR-16/animations.ts (1 hunks)
  • src/shared/configurations/items/SR-16/properties.ts (1 hunks)
  • src/shared/configurations/items/ak_47/animations.ts (1 hunks)
  • src/shared/configurations/items/default/animations.ts (6 hunks)
  • src/shared/configurations/items/tokarev_tt_33/animations.ts (1 hunks)
  • src/shared/getObjectSize.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/client/controllers/viewmodel.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/client/shobnode/ui/ShobnodeOutput.tsx (1)
src/client/shobnode/ui/ShobnodeNode.tsx (1)
  • props (4-12)
src/shared/configurations/items/ak_47/animations.ts (2)
src/shared/configurations/items/ak_47/index.ts (1)
  • animations (12-12)
src/shared/configurations/items/tokarev_tt_33/index.ts (1)
  • animations (12-12)
src/client/controllers/animation.ts (4)
src/client/types/items.ts (1)
  • Viewmodel (7-18)
src/shared/configurations/items/index.ts (2)
  • configs (24-29)
  • ItemConfig (7-7)
src/client/functions/items/create_viewmodel.ts (1)
  • itemName (48-113)
src/client/shobnode/index.tsx (1)
  • log (123-138)
src/client/shobnode/ui/ShobnodeTable.tsx (1)
src/client/shobnode/ui/ShobnodeNode.tsx (1)
  • props (4-12)
src/client/controllers/movement.ts (2)
src/client/controllers/core.ts (1)
  • OnCharacterAdded (5-7)
src/client/controllers/camera.ts (1)
  • SensitivityModifier (75-123)
src/client/controllers/freelook.ts (1)
src/client/controllers/camera.ts (1)
  • Controller (167-300)
src/client/controllers/movement_sounds.ts (1)
src/client/controllers/movement.ts (3)
  • OnJump (15-17)
  • OnLand (19-21)
  • OnRunningChanged (27-29)
src/client/shobnode/index.tsx (1)
src/stories/menu.story.tsx (1)
  • target (9-35)
src/client/items/base_item.ts (4)
src/client/controllers/movement.ts (3)
  • OnJump (15-17)
  • OnRunningChanged (27-29)
  • OnLand (19-21)
src/client/controllers/camera.ts (1)
  • Modifier (14-73)
src/client/functions/items/create_viewmodel.ts (1)
  • itemName (48-113)
src/client/controllers/items.ts (1)
  • equip (53-70)
🔇 Additional comments (8)
src/shared/configurations/items/SR-16/properties.ts (1)

6-6: LGTM! Configuration property addition.

The addition of aimOffset: 2 is straightforward and aligns with the weapon property enhancements in this PR.

src/client/controllers/freelook.ts (1)

11-11: LGTM! Freelook sensitivity adjustments.

The Y-axis clamp tightening (0.4 vs 1.0) and mouse sensitivity increase (270 vs 300 divisor) appear to be intentional camera control tuning changes that align with the PR's camera control enhancements.

Also applies to: 34-34

src/client/types/items.ts (1)

10-13: LGTM! Type definitions extended for animation system.

The addition of CameraBone: Motor6D within Torso and the "Left Arm" / "Right Arm" parts properly extend the Viewmodel interface to support the new centralized Animation controller integration.

src/client/controllers/items.ts (1)

49-49: LGTM! Initialization timing coordination.

The started flag correctly coordinates automatic slot selection between onStart and onCharacterAdded, preventing race conditions and ensuring the initial item equip occurs exactly once after both the controller starts and the character is added.

Also applies to: 103-109, 135-139

src/shared/configurations/items/SR-16/animations.ts (1)

7-7: LGTM! Migration to priority-based animation selection.

The changes consistently replace weight-based selection with priority fields (priority 2 for animations, priority 1 for idle pose) and convert the idle entry to type "Pose". This aligns with the centralized animation controller refactor described in the PR objectives.

Also applies to: 12-12, 17-17, 22-22, 25-27, 33-33

src/client/functions/items/create_viewmodel.ts (1)

9-10: LGTM! Animation controller integration.

The viewmodel creation flow now properly integrates with the centralized Animation controller using dependency injection. The assignViewmodel call is correctly placed after the viewmodel is fully configured but before it's returned.

Also applies to: 52-52, 110-110

src/shared/configurations/items/tokarev_tt_33/animations.ts (1)

7-7: LGTM! Consistent priority-based migration.

The tokarev animations follow the same priority-based pattern as SR-16, ensuring consistency across weapon configurations. The migration from weights to priority fields and the idle-to-Pose conversion are applied uniformly.

Also applies to: 12-12, 17-17, 22-22, 25-27, 33-33

src/client/shobnode/ui/ShobnodeOutput.tsx (1)

18-29: LGTM! Color mapping implementation.

The message type to color mapping logic correctly handles Info (blue), Error (red), and Warning (orange) message types with appropriate color values.

Comment on lines +30 to +51
playAnimation = (animationKey: string): CanimTracks | undefined => {
const animationType = this.typeLookup[animationKey];
if (!animationType) {
throw `Couldn't find type for given animationKey (animationKey: ${animationKey})`;
}

print(this.viewmodelAnimator.identified_bones);
print(this.characterAnimator.identified_bones);

const viewmodelTrack =
(animationType === "Animation" ? this.viewmodelAnimator.play_animation(animationKey) : this.viewmodelAnimator.play_pose(animationKey)) || undefined;
const characterTrack =
(animationType === "Animation" ? this.characterAnimator.play_animation(animationKey) : this.characterAnimator.play_pose(animationKey)) || undefined;

return viewmodelTrack;
};

stopAnimation = (animationKey: string): void => {
this.viewmodelAnimator.stop_animation(animationKey);
this.characterAnimator.stop_animation(animationKey);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Store running tracks so getAnimationTrack works.
We never populate this.tracks, so getAnimationTrack always returns undefined. Persist the tracks when playing and clear them when stopping.

Apply this diff:

 		const viewmodelTrack =
 			(animationType === "Animation" ? this.viewmodelAnimator.play_animation(animationKey) : this.viewmodelAnimator.play_pose(animationKey)) || undefined;
 		const characterTrack =
 			(animationType === "Animation" ? this.characterAnimator.play_animation(animationKey) : this.characterAnimator.play_pose(animationKey)) || undefined;
 
+		this.tracks.viewmodel[animationKey] = viewmodelTrack;
+		this.tracks.character[animationKey] = characterTrack;
+
 		return viewmodelTrack;
 	};
 
 	stopAnimation = (animationKey: string): void => {
 		this.viewmodelAnimator.stop_animation(animationKey);
 		this.characterAnimator.stop_animation(animationKey);
+		this.tracks.viewmodel[animationKey] = undefined;
+		this.tracks.character[animationKey] = undefined;
 	};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
playAnimation = (animationKey: string): CanimTracks | undefined => {
const animationType = this.typeLookup[animationKey];
if (!animationType) {
throw `Couldn't find type for given animationKey (animationKey: ${animationKey})`;
}
print(this.viewmodelAnimator.identified_bones);
print(this.characterAnimator.identified_bones);
const viewmodelTrack =
(animationType === "Animation" ? this.viewmodelAnimator.play_animation(animationKey) : this.viewmodelAnimator.play_pose(animationKey)) || undefined;
const characterTrack =
(animationType === "Animation" ? this.characterAnimator.play_animation(animationKey) : this.characterAnimator.play_pose(animationKey)) || undefined;
return viewmodelTrack;
};
stopAnimation = (animationKey: string): void => {
this.viewmodelAnimator.stop_animation(animationKey);
this.characterAnimator.stop_animation(animationKey);
};
playAnimation = (animationKey: string): CanimTracks | undefined => {
const animationType = this.typeLookup[animationKey];
if (!animationType) {
throw `Couldn't find type for given animationKey (animationKey: ${animationKey})`;
}
print(this.viewmodelAnimator.identified_bones);
print(this.characterAnimator.identified_bones);
const viewmodelTrack =
(animationType === "Animation" ? this.viewmodelAnimator.play_animation(animationKey) : this.viewmodelAnimator.play_pose(animationKey)) || undefined;
const characterTrack =
(animationType === "Animation" ? this.characterAnimator.play_animation(animationKey) : this.characterAnimator.play_pose(animationKey)) || undefined;
this.tracks.viewmodel[animationKey] = viewmodelTrack;
this.tracks.character[animationKey] = characterTrack;
return viewmodelTrack;
};
stopAnimation = (animationKey: string): void => {
this.viewmodelAnimator.stop_animation(animationKey);
this.characterAnimator.stop_animation(animationKey);
this.tracks.viewmodel[animationKey] = undefined;
this.tracks.character[animationKey] = undefined;
};
🤖 Prompt for AI Agents
In src/client/controllers/animation.ts around lines 30 to 51, playAnimation
currently returns only the viewmodel track and never persists running tracks, so
getAnimationTrack always returns undefined; update playAnimation to save the
produced viewmodelTrack and characterTrack into this.tracks (keyed by
animationKey) after creating them, and update stopAnimation to remove/clear the
entry for animationKey (and still call stop_animation on both animators) so
tracks are persisted while running and cleared when stopped.

Comment on lines +95 to +118
let animation: CanimTrack | CanimPose;

if (animationType === "Animation") {
animation = this.viewmodelAnimator.load_animation(key, priority, id);
this.characterAnimator.load_animation(key, priority, id);
} else {
animation = this.viewmodelAnimator.load_pose(key, priority, id);
this.characterAnimator.load_pose(key, priority, id);
}

animation.finished_loading.Wait();
globalChecksum.current += 1;

animation.looped = looped;
// eslint-disable-next-line camelcase
animation.fade_time = fadeTime;

if (weights !== undefined) {
for (const [bone, weight] of pairs(weights)) {
animation.bone_weights[bone] = weight;
}
}

loadedTracks[animationName] = { track: animation, trackType: animationType, rebased };
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Configure character tracks the same as viewmodel tracks.
We drop the track returned by characterAnimator.load_*, so the character-side animations never get their looped, fade_time, or bone_weights configured and may still be loading when we mark the checksum complete. Character idles/runs will therefore stop immediately or blend incorrectly.

Apply this diff:

-					let animation: CanimTrack | CanimPose;
-
-					if (animationType === "Animation") {
-						animation = this.viewmodelAnimator.load_animation(key, priority, id);
-						this.characterAnimator.load_animation(key, priority, id);
-					} else {
-						animation = this.viewmodelAnimator.load_pose(key, priority, id);
-						this.characterAnimator.load_pose(key, priority, id);
-					}
-
-					animation.finished_loading.Wait();
+					let viewmodelAnimation: CanimTracks;
+					let characterAnimation: CanimTracks;
+
+					if (animationType === "Animation") {
+						viewmodelAnimation = this.viewmodelAnimator.load_animation(key, priority, id);
+						characterAnimation = this.characterAnimator.load_animation(key, priority, id);
+					} else {
+						viewmodelAnimation = this.viewmodelAnimator.load_pose(key, priority, id);
+						characterAnimation = this.characterAnimator.load_pose(key, priority, id);
+					}
+
+					viewmodelAnimation.finished_loading.Wait();
+					characterAnimation.finished_loading.Wait();
 					globalChecksum.current += 1;
 
-					animation.looped = looped;
-					// eslint-disable-next-line camelcase
-					animation.fade_time = fadeTime;
+					viewmodelAnimation.looped = looped;
+					characterAnimation.looped = looped;
+					// eslint-disable-next-line camelcase
+					viewmodelAnimation.fade_time = fadeTime;
+					// eslint-disable-next-line camelcase
+					characterAnimation.fade_time = fadeTime;
 
 					if (weights !== undefined) {
 						for (const [bone, weight] of pairs(weights)) {
-							animation.bone_weights[bone] = weight;
+							viewmodelAnimation.bone_weights[bone] = weight;
+							characterAnimation.bone_weights[bone] = weight;
 						}
 					}
 
-					loadedTracks[animationName] = { track: animation, trackType: animationType, rebased };
+					loadedTracks[animationName] = { track: viewmodelAnimation, trackType: animationType, rebased };

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/client/controllers/animation.ts around lines 95-118, the code only
captures and configures the viewmodel animation while calling
characterAnimator.load_* without keeping its return value, so the character
track never gets looped, fade_time or bone_weights configured and may still be
loading when the checksum is bumped; fix this by capturing the character
animation into a separate variable (e.g. characterAnimation) from
characterAnimator.load_animation/load_pose, wait on both
animation.finished_loading and characterAnimation.finished_loading before
incrementing globalChecksum.current, and apply looped, fade_time and any
bone_weights to both the viewmodel animation and the characterAnimation (and
store/configure both as needed).

import { Input } from "./input";
import { UserInputService } from "@rbxts/services";
import { OnCharacterAdded } from "./core";
import { smoothClamp } from "shared/utilities/number_utility";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused import.

The smoothClamp import is added but never used in the code.

Apply this diff to remove the unused import:

-import { smoothClamp } from "shared/utilities/number_utility";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { smoothClamp } from "shared/utilities/number_utility";
🤖 Prompt for AI Agents
In src/client/controllers/freelook.ts around line 6, the file imports
smoothClamp from "shared/utilities/number_utility" but never uses it; remove
that unused import from the import list so the file no longer imports
smoothClamp (and reformat the remaining imports if necessary).

Comment on lines +3 to 43
const animations = {
reload: {
type: "Animation",
id: 14643007671,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 4,
},
equip: {
type: "Animation",
id: 14642999480,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
chamberToReady: {
type: "Animation",
id: 14642991821,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
magCheck: {
type: "Animation",
id: 14643016064,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
idle: {
type: "Animation",
type: "Pose",
id: 14643032549,
priority: 1,
looped: true,
weights: [
[2, 2, 2],
[1, 1, 1],
],
},
chamberCheck: {
type: "Animation",
id: 14643029100,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
run: {
type: "Animation",
id: 14449811036,
priority: 2,
fadeTime: 2,
rebased: false,
looped: true,
weights: [
[1, 1, 1],
[2, 2, 2],
],
},
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Restore WeaponAnimations typing for this config.
Dropping the Partial<WeaponAnimations> annotation removes compile-time validation of the animation keys, so future typos (e.g. idlle) would slip through until runtime. Please keep the explicit type to preserve the guardrails.

Apply this diff:

-const animations = {
+const animations: Partial<WeaponAnimations> = {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const animations = {
reload: {
type: "Animation",
id: 14643007671,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 4,
},
equip: {
type: "Animation",
id: 14642999480,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
chamberToReady: {
type: "Animation",
id: 14642991821,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
magCheck: {
type: "Animation",
id: 14643016064,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
idle: {
type: "Animation",
type: "Pose",
id: 14643032549,
priority: 1,
looped: true,
weights: [
[2, 2, 2],
[1, 1, 1],
],
},
chamberCheck: {
type: "Animation",
id: 14643029100,
weights: [
[2, 2, 2],
[1, 1, 1],
],
priority: 3,
},
run: {
type: "Animation",
id: 14449811036,
priority: 2,
fadeTime: 2,
rebased: false,
looped: true,
weights: [
[1, 1, 1],
[2, 2, 2],
],
},
};
const animations: Partial<WeaponAnimations> = {
reload: {
type: "Animation",
id: 14643007671,
priority: 4,
},
equip: {
type: "Animation",
id: 14642999480,
priority: 3,
},
chamberToReady: {
type: "Animation",
id: 14642991821,
priority: 3,
},
magCheck: {
type: "Animation",
id: 14643016064,
priority: 3,
},
idle: {
type: "Pose",
id: 14643032549,
priority: 1,
looped: true,
},
chamberCheck: {
type: "Animation",
id: 14643029100,
priority: 3,
},
run: {
type: "Animation",
id: 14449811036,
priority: 2,
fadeTime: 2,
rebased: false,
looped: true,
},
};
🤖 Prompt for AI Agents
In src/shared/configurations/items/ak_47/animations.ts around lines 3 to 43, the
explicit WeaponAnimations typing was removed which disables compile-time
validation of animation keys; restore the explicit type by declaring the object
as const animations: Partial<WeaponAnimations> = { ... } (or const animations:
WeaponAnimations = { ... } if all keys present) and add an import for the
WeaponAnimations type at the top of the file if it isn't already imported so the
compiler can validate the keys.

@@ -0,0 +1,7 @@
export default (object: { [key in string]: unknown }) => {
let size = 0;
for (const [value, key] of pairs(object)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix the destructuring order for pairs().

In Lua/Luau, pairs() returns (key, value) not (value, key). The current destructuring order is reversed, which will cause the variables to hold incorrect values (though in this case they're unused, the code is still semantically wrong).

Apply this diff to fix the destructuring order:

-	for (const [value, key] of pairs(object)) {
+	for (const [key, value] of pairs(object)) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (const [value, key] of pairs(object)) {
for (const [key, value] of pairs(object)) {
🤖 Prompt for AI Agents
In src/shared/getObjectSize.ts around line 3, the for-loop destructures
pairs(object) as [value, key] but Lua/Luau's pairs() yields (key, value); swap
the order to [key, value] so the loop variables match the API contract (even if
unused) to keep semantics correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant