feat: configurable relative head/body angle#320
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughIntroduces configurable head rotation angle limits for players with separate values for blocking and normal states. Adds two configuration fields, corresponding UI options, an injected mixin override to apply these limits dynamically, and translation keys. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java`:
- Around line 55-57: The method overrideMaxHeadRoationRelativeToBody should
validate and sanitize the config angles before returning them: fetch
NEABaseMod.config.maxBlockingAngle / maxNormalAngle inside
overrideMaxHeadRoationRelativeToBody, test for Float.isFinite and non-negative
(and optionally clamp to a sensible upper bound like 180f), and if invalid
replace with a safe fallback (e.g., 0f or a known constant default). Update the
return path in overrideMaxHeadRoationRelativeToBody to use the sanitized/clamped
value instead of returning the raw config field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59204f22-9698-4d4f-8583-4cf5fd9da65e
📒 Files selected for processing (4)
NEAVersionless/src/main/java/dev/tr7zw/notenoughanimations/versionless/config/Config.javasrc/main/java/dev/tr7zw/notenoughanimations/config/ConfigScreenProvider.javasrc/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.javasrc/main/resources/assets/notenoughanimations/lang/en_us.json
also add a lil guard thing
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java (1)
59-73: Extract the repeated angle-clamping logic.The blocking and normal branches do the same range-normalization work. A small helper would remove the duplication and clean up the current local naming noise in one pass.
♻️ Possible cleanup
+ private static float clampAngle(float value, float max) { + if (value < 0.0f) { + return 0.0f; + } + if (value > max) { + return max; + } + return value; + } + //? if >= 1.20.4 { `@WrapMethod`(method = "getMaxHeadRotationRelativeToBody") protected float overrideMaxHeadRoationRelativeToBody(Operation<Float> original) { Player player = (Player) (Object) this; - - float blocking_angle = NEABaseMod.config.maxBlockingAngle; - if (blocking_angle < 0) { - blocking_angle = 0; - } else if (blocking_angle > 15) { - blocking_angle = 15; - } - - float normal_angle = NEABaseMod.config.maxNormalAngle; - if (normal_angle < 0) { - normal_angle = 0; - } else if (normal_angle > 50) { - normal_angle = 50; - } - - return player.isBlocking() ? blocking_angle : normal_angle; + float blockingAngle = clampAngle(NEABaseMod.config.maxBlockingAngle, 15.0f); + float normalAngle = clampAngle(NEABaseMod.config.maxNormalAngle, 50.0f); + return player.isBlocking() ? blockingAngle : normalAngle; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java` around lines 59 - 73, Extract the repeated clamp logic into a small helper (e.g., clampAngle or normalizeAngle) and use it for both values: replace the blocking_angle and normal_angle blocks by calling clampAngle(NEABaseMod.config.maxBlockingAngle) and clampAngle(NEABaseMod.config.maxNormalAngle), then return player.isBlocking() ? clampedBlocking : clampedNormal; ensure the helper enforces the same min/max bounds currently used (0..15 for blocking if you keep separate bounds or make the helper accept min/max so you can reuse it for 0..15 and 0..50) and place it in the PlayerEntityMixin class so it can be referenced directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java`:
- Around line 59-73: Extract the repeated clamp logic into a small helper (e.g.,
clampAngle or normalizeAngle) and use it for both values: replace the
blocking_angle and normal_angle blocks by calling
clampAngle(NEABaseMod.config.maxBlockingAngle) and
clampAngle(NEABaseMod.config.maxNormalAngle), then return player.isBlocking() ?
clampedBlocking : clampedNormal; ensure the helper enforces the same min/max
bounds currently used (0..15 for blocking if you keep separate bounds or make
the helper accept min/max so you can reuse it for 0..15 and 0..50) and place it
in the PlayerEntityMixin class so it can be referenced directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09bd626d-d03c-4698-8270-3744fb0e112f
📒 Files selected for processing (2)
gradle-compose.ymlsrc/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java
this does not fix the build issue. waiting till tomorrow to figure that out
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java (1)
56-67: Consider adding a null guard for config access.Based on the codebase context,
NEABaseMod.configis declared without initialization and could theoretically be null if this mixin runs before config loading completes. While unlikely in practice, a defensive null check would prevent potential NPE.🛡️ Suggested defensive check
`@WrapMethod`(method = "getMaxHeadRotationRelativeToBody") protected float overrideMaxHeadRotationRelativeToBody(Operation<Float> original) { + if (NEABaseMod.config == null) { + return original.call(); + } Player player = (Player) (Object) this; // ... validation and returnThis also addresses the unused
originalparameter by providing a fallback path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java` around lines 56 - 67, The mixin method PlayerEntityMixin.overrideMaxHeadRoationRelativeToBody should defensively handle NEABaseMod.config possibly being null and use the provided original Operation<Float> as a fallback; add a null-check for NEABaseMod.config at the start of the method and if it's null return original.call() (or original.apply(...) consistent with Operation API) so you don't NPE, and then proceed to read/clamp NEABaseMod.config.maxBlockingAngle and maxNormalAngle as before; also ensure the unused parameter original is used in the fallback path to preserve expected behavior when config isn't available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java`:
- Around line 59-65: The validation conditions for
NEABaseMod.config.maxBlockingAngle and maxNormalAngle are wrong: replace the &&
with || so out-of-range values (less than 0.0f OR greater than the intended
upper bound) are clamped; decide and set the proper upper bounds (e.g.
15.0f/50.0f or 180.0f) and use those in the clamps for maxBlockingAngle and
maxNormalAngle. Also correct the method name typo
overrideMaxHeadRoationRelativeToBody → overrideMaxHeadRotationRelativeToBody and
address the unused parameter original in that method (either use it to preserve
vanilla behavior when appropriate or remove/annotate it if intentional),
ensuring the method behavior is explicit.
---
Nitpick comments:
In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java`:
- Around line 56-67: The mixin method
PlayerEntityMixin.overrideMaxHeadRoationRelativeToBody should defensively handle
NEABaseMod.config possibly being null and use the provided original
Operation<Float> as a fallback; add a null-check for NEABaseMod.config at the
start of the method and if it's null return original.call() (or
original.apply(...) consistent with Operation API) so you don't NPE, and then
proceed to read/clamp NEABaseMod.config.maxBlockingAngle and maxNormalAngle as
before; also ensure the unused parameter original is used in the fallback path
to preserve expected behavior when config isn't available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8dda45fb-0166-411d-9597-49f07f221f0e
📒 Files selected for processing (1)
src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java
Comment on lines
+59
to
+65
| if (NEABaseMod.config.maxBlockingAngle < 0.0f && NEABaseMod.config.maxBlockingAngle > 15.0f) { | ||
| NEABaseMod.config.maxBlockingAngle = 15.0f; | ||
| } | ||
|
|
||
| if (NEABaseMod.config.maxNormalAngle < 0.0f && NEABaseMod.config.maxNormalAngle > 50.0f) { | ||
| NEABaseMod.config.maxNormalAngle = 50.0f; | ||
| } |
There was a problem hiding this comment.
Logic error: validation conditions will never trigger.
The conditions use && instead of ||, making them always evaluate to false. No value can be both less than 0 AND greater than 15/50 simultaneously, so these guards never apply.
Additionally:
- Method name typo:
overrideMaxHeadRoationRelativeToBody→overrideMaxHeadRotationRelativeToBody - The
originalparameter is unused, meaning the vanilla behavior is completely bypassed (verify this is intentional)
🐛 Proposed fix
`@WrapMethod`(method = "getMaxHeadRotationRelativeToBody")
- protected float overrideMaxHeadRoationRelativeToBody(Operation<Float> original) {
+ protected float overrideMaxHeadRotationRelativeToBody(Operation<Float> original) {
Player player = (Player) (Object) this;
- if (NEABaseMod.config.maxBlockingAngle < 0.0f && NEABaseMod.config.maxBlockingAngle > 15.0f) {
+ if (NEABaseMod.config.maxBlockingAngle < 0.0f || NEABaseMod.config.maxBlockingAngle > 180.0f) {
NEABaseMod.config.maxBlockingAngle = 15.0f;
}
- if (NEABaseMod.config.maxNormalAngle < 0.0f && NEABaseMod.config.maxNormalAngle > 50.0f) {
+ if (NEABaseMod.config.maxNormalAngle < 0.0f || NEABaseMod.config.maxNormalAngle > 180.0f) {
NEABaseMod.config.maxNormalAngle = 50.0f;
}
return player.isBlocking() ? NEABaseMod.config.maxBlockingAngle : NEABaseMod.config.maxNormalAngle;
}Note: I adjusted the upper bound to 180.0f assuming that's a reasonable max rotation angle. If the intent was to clamp to the default values (15/50), use those as upper bounds instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/dev/tr7zw/notenoughanimations/mixins/PlayerEntityMixin.java`
around lines 59 - 65, The validation conditions for
NEABaseMod.config.maxBlockingAngle and maxNormalAngle are wrong: replace the &&
with || so out-of-range values (less than 0.0f OR greater than the intended
upper bound) are clamped; decide and set the proper upper bounds (e.g.
15.0f/50.0f or 180.0f) and use those in the clamps for maxBlockingAngle and
maxNormalAngle. Also correct the method name typo
overrideMaxHeadRoationRelativeToBody → overrideMaxHeadRotationRelativeToBody and
address the unused parameter original in that method (either use it to preserve
vanilla behavior when appropriate or remove/annotate it if intentional),
ensuring the method behavior is explicit.
|
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
2 participants
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.
This PR adds 2 sliders that allow you to configure the relative head/body rotation angle for normal movement and blocking.