Skip to content

Conversation

@tr7zw
Copy link
Owner

@tr7zw tr7zw commented Nov 11, 2025

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

Updates gradle-compose.yml with expanded metadata and dependency placeholders, adds mixinextras flag, and updates supported Minecraft versions to 1.21.1, 1.21.8, and 1.21.10. Refactors codebase to use conditional compilation markers instead of preprocessor directives, introduces new rendering state access interfaces, migrates mixin injection targets from MultiBufferSource to SubmitNodeCollector/AvatarRenderState, and adds dynamic offset handler registration for player rendering.

Changes

Cohort / File(s) Summary
Build Configuration
gradle-compose.yml, settings.json, versions/mainProject
Updated source URL to stonecutter, expanded metadata fields (version, description, URLs, mod IDs, entrypoints, dependencies), added mixinextras flag, updated supported MC versions from 1.21.x/1.21.6.x to 1.21.1 and 1.21.8/1.21.10 variants
Core Logic & API
src/main/java/dev/tr7zw/firstperson/FirstPersonBootstrap.java, src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java, src/main/java/dev/tr7zw/firstperson/FirstPersonModelMod.java
Migrated Forge/NeoForge conditional scaffolding to commented hybrid structure, added isRenderingPlayer static field, introduced version-conditional KeyMapping initialization, wrapped Fabric interface implementation behind fabric flag
Utility & Inventory
src/main/java/dev/tr7zw/firstperson/InventoryUtil.java, src/main/java/dev/tr7zw/firstperson/LogicHandler.java
Replaced direct method implementations with version-guarded blocks using conditional markers, added lookingDown overloads accepting LivingEntity and LivingEntityRenderState, added PlayerOffsetHandlers hook for dynamic offset customization
Access Interfaces
src/main/java/dev/tr7zw/firstperson/access/LivingEntityRenderStateAccess.java, src/main/java/dev/tr7zw/firstperson/access/PlayerModelAccess.java, src/main/java/dev/tr7zw/firstperson/access/PlayerRendererAccess.java, src/main/java/dev/tr7zw/firstperson/access/AgeableListModelAccess.java
Introduced LivingEntityRenderStateAccess interface with camera entity, arm, body visibility, and offset methods; moved getCloak() method behind version-conditional block in PlayerModelAccess; broadened imports in other accessors
Public API
src/main/java/dev/tr7zw/firstperson/api/FirstPersonAPI.java, src/main/java/dev/tr7zw/firstperson/api/PlayerOffsetHandler.java
Consolidated imports to wildcard style, added GetPlayerOffsetHandlers integration, PlayerOffsetHandler interface unchanged
Configuration
src/main/java/dev/tr7zw/firstperson/config/ConfigScreenProvider.java, src/main/java/dev/tr7zw/firstperson/fabric/FPModMenuProvider.java
Updated ConfigScreenProvider to use AvatarRenderState/PlayerRenderState access patterns with version-conditional initialization, migrated FPModMenuProvider from preprocessor guards to inline feature flags with wildcard imports
Mod Support
src/main/java/dev/tr7zw/firstperson/modsupport/FreecamSupport.java, src/main/java/dev/tr7zw/firstperson/modsupport/ModSupportLoader.java, src/main/java/dev/tr7zw/firstperson/modsupport/PlayerAnimatorSupport.java
Replaced specific imports with wildcard imports, disabled PlayerAnimatorSupport core logic via commenting, added Freecam.isEnabled() call in constructor
Forge Rendering
src/main/java/dev/tr7zw/firstperson/forge/RenderHandEventListener.java
Migrated from preprocessor blocks to environment-conditional structure with explicit Forge event type qualification
Mixins: Rendering State & Accessors
src/main/java/dev/tr7zw/firstperson/mixins/EntityAccessor.java, src/main/java/dev/tr7zw/firstperson/mixins/AgeableListModelMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java
Added EntityAccessor and LivingEntityRenderStateMixin with field accessors and visibility/offset methods, retargeted AgeableListModelMixin from ModelPart to AgeableListModel, added HumanoidModelMixin to inject visibility/offset logic at setupAnim
Mixins: Item & Hand Rendering
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/HeldItemRendererMixin.java
Updated injection targets to submitArmWithItem/renderFirstPersonItem with SubmitNodeCollector parameter, introduced LivingEntityRenderStateAccess-based arm visibility checks, added deltaTick parameter naming
Mixins: Entity & Feature Rendering
src/main/java/dev/tr7zw/firstperson/mixins/FishingBobberRendererMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java
Migrated render injections to submit-based signatures using SubmitNodeCollector and new render state types, added version-conditional scaffolding with comment-wrapped alternative paths
Mixins: Feature & Model Rendering
src/main/java/dev/tr7zw/firstperson/mixins/ArmorFeatureRendererMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/FeatureRendererMixin.java
Disabled core render logic via commented blocks, introduced generic EntityModel signatures with float color parameters, added version-specific VillagerHeadModel handling; FeatureRendererMixin includes second public class definition
Mixins: Model & Layer Handling
src/main/java/dev/tr7zw/firstperson/mixins/ModelPartMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/PlayerModelMixin.java
Disabled copyFrom injection via commenting for < 1.21.9, conditionally compiled getCloak() method behind version guard, retained setPos injection
Mixins: Player & World Rendering
src/main/java/dev/tr7zw/firstperson/mixins/PlayerMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java
Retargeted PlayerMixin conditionally to LivingEntity (>= 1.21.5), retargeted PlayerRendererMixin to AvatarRenderer with extractRenderState injection replacing getRenderOffset, updated constructor signature, consolidated LivingEntityRendererMixin version guards
Mixins: Rendering Infrastructure
src/main/java/dev/tr7zw/firstperson/mixins/RenderDispatcherMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/WorldRendererMixin.java, src/main/java/dev/tr7zw/firstperson/mixins/StuckInBodyLayerMixin.java
RenderDispatcherMixin added shadowMove injection with post-render offset handling, WorldRendererMixin changed to abstract with extractEntity shadow and extractVisibleEntities injection, StuckInBodyLayerMixin migrated to submit-based signature with AvatarRenderState
Resources & Tests
src/main/resources/firstperson.mixins.json, src/test/java/dev/tr7zw/tests/MixinTests.java
Reorganized client mixin list with new entries (EntityAccessor, LivingEntityRenderStateMixin, HumanoidModelMixin) and repositioned existing ones, added testInjectedInterfaces() and testMixins() test methods with @BeforeAll/@test annotations

Sequence Diagram

sequenceDiagram
    actor Render as Rendering Pipeline
    participant FPCore as FirstPersonModelCore
    participant Handler as LogicHandler
    participant Access as LivingEntityRenderStateAccess
    participant Offset as PlayerOffsetHandler[]

    Render->>Handler: updatePositionOffset(entity, offset)
    Handler->>Handler: Compute base offset
    Handler->>Offset: For each registered handler
    Offset->>Handler: applyOffset(player, delta, pos, offset)
    Handler->>Handler: Updated offset
    
    Render->>FPCore: isRenderingPlayer check
    FPCore-->>Render: rendering context
    
    Render->>Access: Get visibility state
    Access-->>Render: hideArms, hideBody, armOffset
    
    Render->>Render: Apply offset & visibility
    Render->>Render: Render with adjusted state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Areas requiring extra attention:

  • Mixin target and injection signature changes (PlayerRendererMixin, HeldItemFeatureRendererMixin, ElytraLayerMixin, RenderDispatcherMixin, WorldRendererMixin) — verify correct method descriptors and parameter mapping for new SubmitNodeCollector/AvatarRenderState types across multiple MC versions
  • New rendering state access interface integration (LivingEntityRenderStateAccess, LivingEntityRenderStateMixin) — ensure state fields are properly persisted and accessed across render cycles, verify thread safety and state lifecycle
  • Conditional compilation marker syntax (//? if/else blocks) — audit all version-guarded code paths, particularly in PlayerAnimatorSupport, LogicHandler, ConfigScreenProvider to ensure correct branching logic and fallback paths
  • PlayerOffsetHandlers hook registration and application (LogicHandler.updatePositionOffset, FirstPersonAPI) — verify handler iteration and offset composition logic, check for null pointer exceptions and handler ordering
  • Migration from ModelPart to AgeableListModel mixin target (AgeableListModelMixin) — confirm interface implementation completeness and that headParts() delegation is correct
  • Boolean return semantics in render event handlers (RenderHandEventListener) — verify Forge vs. NeoForge handler cancellation logic and return values align with event expectations

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.85% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether the content relates to the changeset. Add a description explaining the purpose of the version bump to 1.21.10, including any migration notes or breaking changes for developers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title '1.21.10' directly corresponds to the main change: updating the project version and Minecraft compatibility to 1.21.10, as shown by version updates in multiple files.
✨ 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 WIP1.21.10

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.

@sonarqubecloud
Copy link

Copy link

@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: 6

Caution

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

⚠️ Outside diff range comments (2)
src/main/java/dev/tr7zw/firstperson/modsupport/PlayerAnimatorSupport.java (1)

18-152: PlayerAnimator offset handling is now a no-op—breaks integration.

All of the transformation work that adjusts the first-person camera based on Player Animator state is commented out, so applyOffset just returns current. This completely disables the PlayerAnimator support path, meaning players with active animations lose their offset and the feature regresses. Please restore the offset computation (or implement the new PartKey access you hinted at) so the handler still applies the animation transforms before returning.

src/main/java/dev/tr7zw/firstperson/mixins/ArmorFeatureRendererMixin.java (1)

24-100: Do not ship the armor-layer mixin as a no-op.

By wrapping the entire injection block in /* ... */, this mixin stops intercepting shouldRender, setPartVisibility, and renderArmorPiece, so the client will always render helmets/arm arms in first-person again—a straight regression of the mod’s core behavior in 1.21.x. Please restore the mixin logic (and the missing ModelPartBase import) or otherwise wire the new renderer-state path before merging.

🧹 Nitpick comments (7)
src/main/java/dev/tr7zw/firstperson/modsupport/FreecamSupport.java (1)

3-4: Consider using explicit imports instead of wildcards.

Wildcard imports reduce code clarity by hiding which specific classes are in use. Explicit imports make dependencies immediately visible and help prevent potential naming conflicts.

Consider this refactor:

-import dev.tr7zw.firstperson.api.*;
-import net.xolt.freecam.*;
+import dev.tr7zw.firstperson.api.ActivationHandler;
+import net.xolt.freecam.Freecam;
src/main/java/dev/tr7zw/firstperson/forge/RenderHandEventListener.java (1)

35-39: Fix inconsistent indentation.

The indentation in this method is inconsistent with the rest of the file and appears to mix tabs and spaces.

Apply this diff to normalize the indentation:

-    //? } else {
-    
-      	public void onRender(RenderHandEvent e) {
-      		if(FirstPersonModelCore.instance.isEnabled() && !FirstPersonModelCore.instance.getLogicHandler().showVanillaHands()) {
-      			e.setCanceled(true);
-      		}
-      	}
+    //? } else {
+
+    public void onRender(RenderHandEvent e) {
+        if(FirstPersonModelCore.instance.isEnabled() && !FirstPersonModelCore.instance.getLogicHandler().showVanillaHands()) {
+            e.setCanceled(true);
+        }
+    }
src/main/java/dev/tr7zw/firstperson/mixins/EntityAccessor.java (1)

11-15: Misleading accessor method naming.

The entityCulling$ prefix suggests these methods are related to entity culling functionality, but the usage in WorldRendererMixin shows they're for first-person rendering position offsets. Consider renaming to align with the mod's purpose, such as firstperson$getRawPosition() and firstperson$setRawPosition(Vec3 position) for clarity.

Apply this diff to improve naming:

-    @Accessor("position")
-    Vec3 entityCulling$getRawPosition();
+    @Accessor("position")
+    Vec3 firstperson$getRawPosition();

-    @Accessor("position")
-    void entityCulling$setRawPosition(Vec3 position);
+    @Accessor("position")
+    void firstperson$setRawPosition(Vec3 position);

Note: This will require updating the call sites in WorldRendererMixin.java.

src/main/java/dev/tr7zw/firstperson/access/PlayerRendererAccess.java (1)

3-5: Consider explicit imports over wildcards.

The shift to wildcard imports (import net.minecraft.client.renderer.entity.layers.*; and import java.util.*;) reduces clarity about which specific classes are used and can lead to naming conflicts. Consider keeping explicit imports unless wildcard usage is an established project standard.

Apply this diff if explicit imports are preferred:

-import net.minecraft.client.renderer.entity.layers.*;
+import net.minecraft.client.renderer.entity.layers.RenderLayer;

-import java.util.*;
+import java.util.List;
src/main/java/dev/tr7zw/firstperson/modsupport/ModSupportLoader.java (1)

3-6: Consider explicit imports over wildcards.

Multiple wildcard imports make it unclear which specific classes are actually used from each package. If explicit imports are preferred in the project, consider reverting to specific imports for better clarity.

src/main/java/dev/tr7zw/firstperson/api/FirstPersonAPI.java (1)

3-5: Consider explicit imports over wildcards.

The wildcard imports reduce clarity about which classes are actually used. If the project prefers explicit imports, consider specifying individual classes like FirstPersonModelCore, ArrayList, and List.

src/main/java/dev/tr7zw/firstperson/LogicHandler.java (1)

279-285: Prefer using the entity-sensitive dynamic toggle.

Now that we accept a LivingEntity, consider delegating to dynamicHandsEnabled(livingEntity) instead of the zero-arg form so the check doesn’t implicitly depend on client.player. It keeps the helper honest if we ever pass in avatars or other proxies.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca76572 and c03e8cd.

📒 Files selected for processing (41)
  • gradle-compose.yml (2 hunks)
  • settings.json (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/FirstPersonBootstrap.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (3 hunks)
  • src/main/java/dev/tr7zw/firstperson/FirstPersonModelMod.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/InventoryUtil.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/LogicHandler.java (5 hunks)
  • src/main/java/dev/tr7zw/firstperson/access/AgeableListModelAccess.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/access/LivingEntityRenderStateAccess.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/access/PlayerModelAccess.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/access/PlayerRendererAccess.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/api/FirstPersonAPI.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/api/PlayerOffsetHandler.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/config/ConfigScreenProvider.java (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/fabric/FPModMenuProvider.java (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/forge/RenderHandEventListener.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/AgeableListModelMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/ArmorFeatureRendererMixin.java (6 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/EntityAccessor.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/FeatureRendererMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/FishingBobberRendererMixin.java (3 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/HeldItemRendererMixin.java (3 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (8 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/ModelPartMixin.java (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/PlayerMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/PlayerModelMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/RenderDispatcherMixin.java (3 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/StuckInBodyLayerMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/mixins/WorldRendererMixin.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/modsupport/FreecamSupport.java (2 hunks)
  • src/main/java/dev/tr7zw/firstperson/modsupport/ModSupportLoader.java (1 hunks)
  • src/main/java/dev/tr7zw/firstperson/modsupport/PlayerAnimatorSupport.java (2 hunks)
  • src/main/resources/firstperson.mixins.json (1 hunks)
  • src/test/java/dev/tr7zw/tests/MixinTests.java (2 hunks)
  • versions/mainProject (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
src/main/java/dev/tr7zw/firstperson/mixins/EntityAccessor.java (1)
src/main/java/dev/tr7zw/firstperson/mixins/WorldRendererMixin.java (1)
  • Mixin (30-131)
src/main/java/dev/tr7zw/firstperson/LogicHandler.java (1)
src/main/java/dev/tr7zw/firstperson/api/FirstPersonAPI.java (1)
  • FirstPersonAPI (7-82)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (3)
src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (1)
  • Mixin (22-58)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1)
  • Mixin (13-42)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (3)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1)
  • Mixin (16-45)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (1)
  • Mixin (27-213)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (7)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1)
  • Mixin (16-45)
src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (1)
  • Mixin (22-58)
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java (1)
  • Mixin (36-84)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (1)
  • Mixin (27-213)
src/main/java/dev/tr7zw/firstperson/mixins/StuckInBodyLayerMixin.java (1)
  • Mixin (18-47)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1)
  • Mixin (30-102)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1)
  • Mixin (13-42)
src/main/java/dev/tr7zw/firstperson/mixins/WorldRendererMixin.java (3)
src/main/java/dev/tr7zw/firstperson/mixins/RenderDispatcherMixin.java (1)
  • Mixin (30-141)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (1)
  • FirstPersonModelCore (15-136)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (5)
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemRendererMixin.java (1)
  • Mixin (22-124)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (1)
  • Mixin (27-213)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1)
  • Mixin (13-42)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (1)
  • FirstPersonModelCore (15-136)
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java (1)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (1)
  • FirstPersonModelCore (15-136)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (12)
src/main/java/dev/tr7zw/firstperson/InventoryUtil.java (1)
  • InventoryUtil (8-60)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1)
  • Mixin (16-45)
src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (1)
  • Mixin (22-58)
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java (1)
  • Mixin (36-84)
src/main/java/dev/tr7zw/firstperson/mixins/HeldItemRendererMixin.java (1)
  • Mixin (22-124)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerModelMixin.java (1)
  • Mixin (14-29)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerMixin.java (1)
  • Mixin (13-38)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1)
  • Mixin (30-102)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1)
  • Mixin (13-42)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/ModelPartMixin.java (1)
  • Mixin (9-62)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (1)
  • FirstPersonModelCore (15-136)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerModelMixin.java (1)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (1)
  • Mixin (27-213)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (2)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1)
  • Mixin (30-102)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/StuckInBodyLayerMixin.java (3)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1)
  • Mixin (16-45)
src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1)
  • Mixin (30-102)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/AgeableListModelMixin.java (1)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRendererMixin.java (1)
  • Mixin (27-213)
src/main/java/dev/tr7zw/firstperson/mixins/FeatureRendererMixin.java (4)
src/main/java/dev/tr7zw/firstperson/mixins/CustomHeadLayerMixin.java (1)
  • Mixin (16-45)
src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java (1)
  • Mixin (13-42)
src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java (1)
  • Mixin (8-71)
src/main/java/dev/tr7zw/firstperson/mixins/ModelPartMixin.java (1)
  • Mixin (9-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (14)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelMod.java (2)

3-6: LGTM!

The conditional import block is properly structured and will correctly include the Fabric API import only when the fabric flag is defined.


8-12: LGTM!

The conditional interface implementation is properly structured and will correctly implement ClientModInitializer only when the fabric flag is defined.

src/main/java/dev/tr7zw/firstperson/forge/RenderHandEventListener.java (2)

1-20: Conditional compilation structure is complex but appears intentional.

The custom preprocessor syntax with /^, ^/, and //? markers creates version-specific code paths for Forge/NeoForge. While this approach adds complexity, it aligns with the broader refactor described in the PR summary to use environment-conditional blocks instead of preprocessor directives.


27-28: No issues found—code initialization is safe.

The code is null-safe by design. The instance field is set in the constructor (line 35), and logicHandler is initialized at the start of sharedSetup() (line 40) before the event listener is registered (lines 57–58). The lifecycle ensures both are non-null when events fire.

src/main/java/dev/tr7zw/firstperson/mixins/PlayerModelMixin.java (1)

7-27: LGTM! Clean version-guarding implementation.

The version-guarded approach correctly isolates the cloak field access for MC versions < 1.21.3, aligning with the corresponding changes in PlayerModelAccess and usage patterns in LivingEntityRendererMixin.

src/main/java/dev/tr7zw/firstperson/InventoryUtil.java (1)

10-58: LGTM! Version guards correctly handle Minecraft API evolution.

The version-guarded implementations properly handle API changes across different Minecraft versions (1.17.0 and 1.21.5), ensuring compatibility with both the old field access patterns and the newer getter methods.

src/main/java/dev/tr7zw/firstperson/access/PlayerModelAccess.java (1)

3-13: LGTM! Interface correctly aligned with implementation.

The version-guarded interface declaration properly matches the conditional implementation in PlayerModelMixin, maintaining API consistency across supported Minecraft versions.

src/test/java/dev/tr7zw/tests/MixinTests.java (3)

20-24: Good: Bootstrap initialization for test setup.

The setup method properly initializes the Minecraft environment required for mixin testing.


26-30: Good: Interface injection verification.

The test correctly verifies that the ModelPartBase interface is injected into ModelPart via mixins using Objenesis for constructor-less instantiation.


32-51: Good: Version-aware mixin smoke tests.

The test appropriately handles the PlayerRenderer → AvatarRenderer transition in MC 1.21.9+ and provides basic verification that mixins can be applied to key renderer classes.

src/main/java/dev/tr7zw/firstperson/mixins/StuckInBodyLayerMixin.java (1)

21-44: Camera-state guard looks solid.

Switching to AvatarRenderState and the isCameraEntity flag cleanly restricts the cancel to the local view while still honoring the config toggle. Looks good.

src/main/java/dev/tr7zw/firstperson/mixins/HeldItemFeatureRendererMixin.java (1)

39-74: State-aware arm skip matches renderer flow.

The mixin now respects the render-state hide flags and the new lookingDown(state) helper, so we only short-circuit when both arms are hidden in first-person. Nice alignment with the Avatar pipeline.

src/main/java/dev/tr7zw/firstperson/mixins/PlayerRendererMixin.java (1)

51-77: extractRenderState augmentation fits the new pipeline.

Gating everything behind isCameraEntity() and pushing arm/body visibility onto the render state mirrors the old behaviour while aligning with the SubmitNodeCollector flow. No issues spotted.

src/main/java/dev/tr7zw/firstperson/mixins/ElytraLayerMixin.java (1)

45-55: Swimming check matches state flags.

The new isCameraEntity + isVisuallySwimming gate reproduces the old Elytra suppression without relying on direct player lookups. Looks good.

Comment on lines +15 to 21
//? if fabric {

@Override
//#endif
//? }
public void onInitializeClient() {
sharedSetup();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Method body must be included in the conditional compilation block.

The conditional block on lines 15-18 only wraps the @Override annotation, but the method declaration and body (lines 19-21) remain unconditionally compiled. When the fabric flag is not defined, this creates an orphaned public method that:

  • Won't be called by Fabric's initialization system (since ClientModInitializer isn't implemented)
  • Exists as dead code with no purpose
  • Contradicts the stated goal of conditionally including Fabric-specific code

Apply this diff to fix the issue:

 //? if fabric {
 
 @Override
-//? }
 public void onInitializeClient() {
     sharedSetup();
 }
+//? }

This ensures the entire method is only present when the fabric flag is defined, consistent with the pattern used for the import and interface implementation blocks.

📝 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
//? if fabric {
@Override
//#endif
//? }
public void onInitializeClient() {
sharedSetup();
}
//? if fabric {
@Override
public void onInitializeClient() {
sharedSetup();
}
//? }
🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/FirstPersonModelMod.java around lines 15
to 21, the conditional compilation only wraps the @Override annotation leaving
the onInitializeClient() method and its body unguarded; move the entire method
declaration and body inside the same fabric conditional block so the method
(including @Override, signature and body calling sharedSetup()) is only compiled
when the fabric flag is defined, ensuring matching opening/closing conditional
markers and preserving indentation.

Comment on lines 8 to 10
@Mixin(ModelPart.class)
public abstract class AgeableListModelMixin {
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Missing AgeableListModelAccess breaks head-part hiding

On 1.21.10 the active branch mixes into ModelPart with an empty body, so AgeableListModel no longer implements AgeableListModelAccess. The LivingEntityRendererMixin check (model instanceof AgeableListModelAccess) now always fails, meaning ageable models keep rendering their head parts in first person. Please restore the mixin on AgeableListModel and re-expose headParts() via the accessor.

Apply this diff to reinstate the accessor for the >=1.21.3 branch:

-import net.minecraft.client.model.geom.*;
+import dev.tr7zw.firstperson.access.*;
+import net.minecraft.client.model.*;
+import net.minecraft.client.model.geom.*;
 import org.spongepowered.asm.mixin.*;
 
-//? if >= 1.21.3 {
-
-@Mixin(ModelPart.class)
-public abstract class AgeableListModelMixin {
-}
+//? if >= 1.21.3 {
+
+@Mixin(AgeableListModel.class)
+public abstract class AgeableListModelMixin implements AgeableListModelAccess {
+
+    @Override
+    public Iterable<ModelPart> firstPersonHeadPartsGetter() {
+        return headParts();
+    }
+
+    @Shadow
+    public abstract Iterable<ModelPart> headParts();
+}
🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/mixins/AgeableListModelMixin.java around
lines 8 to 10, the mixin currently targets ModelPart with an empty body which
removes the AgeableListModelAccess accessor and breaks head-part hiding; change
the mixin target to AgeableListModel (mix into AgeableListModel.class) and
reintroduce the accessor by adding an @Accessor method that exposes the
headParts() field (returning the List<ModelPart>) so AgeableListModel again
implements AgeableListModelAccess and LivingEntityRendererMixin’s instanceof
check succeeds.

Comment on lines +33 to +41
private void setupAnim(HumanoidRenderState renderState, CallbackInfo ci) {
LivingEntityRenderStateAccess access = (LivingEntityRenderStateAccess) renderState;
head.visible = !access.isCameraEntity();
leftArm.visible = !access.hideLeftArm();
leftArm.xRot += access.getArmOffset();
rightArm.visible = !access.hideRightArm();
rightArm.xRot += access.getArmOffset();
body.visible = !access.hideBody();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reset render-state flags before reuse.

LivingEntityRenderState objects are pooled by the new renderer, so the booleans and arm offset you read here stick around between renders. Once PlayerRendererMixin sets hideLeftArm, hideRightArm, hideBody, or a non-zero armOffset, those values stay latched and this mixin keeps hiding/offsetting limbs even after the player drops the item, stops swimming, or toggles dynamic hands off. Please clear the state at the start of PlayerRendererMixin.extractRenderState (e.g., set both arm flags, the body flag, and the offset back to their neutral values) before applying the per-frame conditions. Otherwise the model remains in the hidden/offset state indefinitely.

🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/mixins/HumanoidModelMixin.java around
lines 33-41 the mixin reads hide/offset flags from a pooled
LivingEntityRenderState which can persist between frames; to fix, reset the
render-state booleans and arm offset to their neutral values at the start of
PlayerRendererMixin.extractRenderState (set hideLeftArm=false,
hideRightArm=false, hideBody=false and armOffset=0 (or equivalent neutral value)
before computing per-frame conditions) so pooled objects do not carry over
previous frame settings.

Comment on lines +22 to +69
@Override
public void setIsCameraEntity(boolean value) {
this.cameraEntity = value;
}

@Override
public boolean isCameraEntity() {
return this.cameraEntity;
}

@Override
public void setHideLeftArm(boolean value) {
this.hideLeftArm = value;
}

@Override
public boolean hideLeftArm() {
return this.hideLeftArm;
}

@Override
public void setHideRightArm(boolean value) {
this.hideRightArm = value;
}

@Override
public boolean hideRightArm() {
return this.hideRightArm;
}

@Override
public void setHideBody(boolean value) {
this.hideBody = value;
}

@Override
public boolean hideBody() {
return this.hideBody;
}

@Override
public void setArmOffset(float offset) {
this.armOffset = offset;
}

@Override
public float getArmOffset() {
return this.armOffset;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reset render-state flags on each reuse.
LivingEntityRenderState instances are pooled and reused every frame. Because these @unique booleans/offset never get cleared, once a render path sets something like hideLeftArm or armOffset, it stays set on the next extraction even when the logic handler wants the limbs/body visible again. Players end up with permanently hidden arms/body after any frame that toggles them. Please reset all custom flags when setIsCameraEntity runs so every extract starts from clean defaults.

Apply this diff:

     @Override
     public void setIsCameraEntity(boolean value) {
         this.cameraEntity = value;
+        this.hideLeftArm = false;
+        this.hideRightArm = false;
+        this.hideBody = false;
+        this.armOffset = 0f;
     }
📝 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
@Override
public void setIsCameraEntity(boolean value) {
this.cameraEntity = value;
}
@Override
public boolean isCameraEntity() {
return this.cameraEntity;
}
@Override
public void setHideLeftArm(boolean value) {
this.hideLeftArm = value;
}
@Override
public boolean hideLeftArm() {
return this.hideLeftArm;
}
@Override
public void setHideRightArm(boolean value) {
this.hideRightArm = value;
}
@Override
public boolean hideRightArm() {
return this.hideRightArm;
}
@Override
public void setHideBody(boolean value) {
this.hideBody = value;
}
@Override
public boolean hideBody() {
return this.hideBody;
}
@Override
public void setArmOffset(float offset) {
this.armOffset = offset;
}
@Override
public float getArmOffset() {
return this.armOffset;
@Override
public void setIsCameraEntity(boolean value) {
this.cameraEntity = value;
this.hideLeftArm = false;
this.hideRightArm = false;
this.hideBody = false;
this.armOffset = 0f;
}
@Override
public boolean isCameraEntity() {
return this.cameraEntity;
}
@Override
public void setHideLeftArm(boolean value) {
this.hideLeftArm = value;
}
@Override
public boolean hideLeftArm() {
return this.hideLeftArm;
}
@Override
public void setHideRightArm(boolean value) {
this.hideRightArm = value;
}
@Override
public boolean hideRightArm() {
return this.hideRightArm;
}
@Override
public void setHideBody(boolean value) {
this.hideBody = value;
}
@Override
public boolean hideBody() {
return this.hideBody;
}
@Override
public void setArmOffset(float offset) {
this.armOffset = offset;
}
@Override
public float getArmOffset() {
return this.armOffset;
🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/mixins/LivingEntityRenderStateMixin.java
around lines 22 to 69, setIsCameraEntity currently only sets cameraEntity but
pooled LivingEntityRenderState instances retain prior flags; update
setIsCameraEntity to reset all custom render-state fields (hideLeftArm,
hideRightArm, hideBody) to false and armOffset to 0.0f (and any other custom
@Unique flags in this class) so each reuse starts from clean defaults before
further setters are applied.

Comment on lines +47 to +55
//? if < 1.21.9 {
/*@Inject(method = "copyFrom", at = @At("RETURN"))
public void copyTransform(ModelPart modelPart, CallbackInfo info) {
if (moved) {
zCopy = z;
z = 5000;
}
}
*///? }
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Re-enable the copyFrom hook to keep hidden parts hidden.

Commenting out the @Inject on ModelPart.copyFrom means every call now restores the original pivot/visibility, so any limb we hid via setHidden() pops straight back as soon as copyFrom runs (which happens frequently when armor models sync state). This regresses the arm/armor hiding feature. Please reinstate the hook (or its 1.21.10-safe equivalent) so that we still force z = 5000 whenever moved is true. A direct fix would look like:

-    //? if < 1.21.9 {
-    /*@Inject(method = "copyFrom", at = @At("RETURN"))
-    public void copyTransform(ModelPart modelPart, CallbackInfo info) {
-        if (moved) {
-            zCopy = z;
-            z = 5000;
-        }
-    }
-    *///? }
+    //? if < 1.21.9 {
+    @Inject(method = "copyFrom", at = @At("RETURN"))
+    private void copyTransform(ModelPart modelPart, CallbackInfo info) {
+        if (moved) {
+            zCopy = z;
+            z = 5000;
+            visible = false;
+        }
+    }
+    //? }
📝 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
//? if < 1.21.9 {
/*@Inject(method = "copyFrom", at = @At("RETURN"))
public void copyTransform(ModelPart modelPart, CallbackInfo info) {
if (moved) {
zCopy = z;
z = 5000;
}
}
*///? }
//? if < 1.21.9 {
@Inject(method = "copyFrom", at = @At("RETURN"))
private void copyTransform(ModelPart modelPart, CallbackInfo info) {
if (moved) {
zCopy = z;
z = 5000;
visible = false;
}
}
//? }
🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/mixins/ModelPartMixin.java around lines
47 to 55, the inject for ModelPart.copyFrom was commented out causing hidden
parts to become un-hidden when copyFrom runs; re-enable the hook (or replace
with a 1.21.10-safe injection) so that after copyFrom returns, if moved is true
you set zCopy = z and then force z = 5000; implement this as a RETURN injection
that checks moved and adjusts z/zCopy accordingly, ensuring the annotation and
method signature match the target Minecraft version and mixin mapping.

Comment on lines +36 to +37
//? if >= 1.21.9 {

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Port the shadow/hitbox adjustments to the 1.21.9+ branch

The new >= 1.21.9 branch is empty, so on 1.21.10 this mixin no longer injects into renderShadow/renderHitbox. As a result, first-person post-render offsets aren’t applied: the player shadow stays misplaced and the hitbox isn’t suppressed. Please port the logic from the 1.21.3–1.21.8 block (or otherwise implement the equivalent hooks for the new renderer APIs) so the offset corrections still run on 1.21.10.

🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/mixins/RenderDispatcherMixin.java around
lines 36-37, the >= 1.21.9 branch is empty so renderShadow/renderHitbox logic
isn’t applied on 1.21.10; copy/port the shadow and hitbox adjustment logic from
the 1.21.3–1.21.8 block into this >= 1.21.9 branch (or adapt it to the new
renderer API) by adding the same injection points or equivalent hooks that apply
first-person post-render offsets, ensure the injections target the new renderer
method names/signatures introduced in 1.21.9+, and preserve the suppression of
hitbox rendering and the position/offset corrections used previously.

@tr7zw tr7zw merged commit b78bb34 into main Nov 11, 2025
5 checks passed
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.

3 participants