Add configurable model options #572
Open
Conversation
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
WalkthroughAdds new configuration fields controlling offsets and render toggles, updates LogicHandler to respect these flags for rendering and position offset behavior, extends the config UI to expose the new options, and adds English/Chinese localization strings for all new settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Player
participant LogicHandler
participant Config
participant Renderer
Player->>LogicHandler: onTick()
LogicHandler->>Config: read modifyPlayerModel
alt modifyPlayerModel == true
LogicHandler->>Config: read swimXOffset, crawlXOffset, swimOrCrawlY
LogicHandler->>LogicHandler: compute offsets (state: swim/crawl)
else
LogicHandler-->>LogicHandler: skip position offset update
end
Renderer->>LogicHandler: shouldRenderModel(state, scoping?)
LogicHandler->>Config: read keepModelVisible
alt keepModelVisible == true
LogicHandler-->>Renderer: render model
else
LogicHandler->>Config: read per-state flags (sleeping, spinAttack, flying, swimTransition, scoping)
alt state allowed by its flag
LogicHandler-->>Renderer: render model
else
LogicHandler-->>Renderer: skip rendering model
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![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
🧹 Nitpick comments (6)
src/main/resources/assets/firstperson/lang/en_us.json (1)
32-55: Tighten wording; fix spacing; align tooltip with actual behavior
- Missing space before the parenthesis in the swimming tooltip.
- Label can be made clearer: “Swim/Crawl Y Offset” reads better.
- Tooltip for “Keep Model Visible” mentions dynamic mode, but LogicHandler doesn't tie this flag to dynamicMode; it gates per-state model visibility. The current tooltip is misleading.
Apply this diff to improve clarity and correctness:
- "text.firstperson.option.firstperson.vanillaHandsSkipSwimming.tooltip": "Disables the Vanilla Hands while swimming(also ignoring the dynamic mode)", + "text.firstperson.option.firstperson.vanillaHandsSkipSwimming.tooltip": "Disables the Vanilla Hands while swimming (also ignores Dynamic Mode)", - "text.firstperson.option.firstperson.swimOrCrawlY": "Enable Swim Or Crawl Y Offset", + "text.firstperson.option.firstperson.swimOrCrawlY": "Enable Swim/Crawl Y Offset", - "text.firstperson.option.firstperson.keepModelVisible.tooltip": "Whether to keep the model visible (except in dynamic mode)" + "text.firstperson.option.firstperson.keepModelVisible.tooltip": "Whether to keep the model visible regardless of per-state visibility settings"If “keepModelVisible” is intended to interact with dynamicMode elsewhere, please point me to that code and I’ll adjust the wording accordingly. Otherwise, the above matches LogicHandler’s checks.
src/main/java/dev/tr7zw/firstperson/config/ConfigScreenProvider.java (1)
59-92: Reduce repetition and localize config access; minor UX suggestion
- Repeated lambda access to FirstPersonModelCore.instance.getConfig() hurts readability and is easy to mistype. Pull a local reference.
- Optional: Rename the Chinese comment “渲染开关” to English to keep code comments consistent.
Apply this refactor within this block:
- // NEW - options.add(getIntOption("text.firstperson.option.firstperson.swimXOffset", -100, 100, - () -> FirstPersonModelCore.instance.getConfig().swimXOffset, - i -> FirstPersonModelCore.instance.getConfig().swimXOffset = i)); + // NEW + final var cfg = FirstPersonModelCore.instance.getConfig(); + options.add(getIntOption("text.firstperson.option.firstperson.swimXOffset", -100, 100, + () -> cfg.swimXOffset, i -> cfg.swimXOffset = i)); options.add(getIntOption("text.firstperson.option.firstperson.crawlXOffset", -100, 100, - () -> FirstPersonModelCore.instance.getConfig().crawlXOffset, - i -> FirstPersonModelCore.instance.getConfig().crawlXOffset = i)); + () -> cfg.crawlXOffset, i -> cfg.crawlXOffset = i)); options.add(getOnOffOption("text.firstperson.option.firstperson.swimOrCrawlY", - () -> FirstPersonModelCore.instance.getConfig().swimOrCrawlY, - b -> FirstPersonModelCore.instance.getConfig().swimOrCrawlY = b)); - // 渲染开关 + () -> cfg.swimOrCrawlY, b -> cfg.swimOrCrawlY = b)); + // Render toggles options.add(getOnOffOption("text.firstperson.option.firstperson.renderSleepingModel", - () -> FirstPersonModelCore.instance.getConfig().renderSleepingModel, - b -> FirstPersonModelCore.instance.getConfig().renderSleepingModel = b)); + () -> cfg.renderSleepingModel, b -> cfg.renderSleepingModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.renderSpinAttackModel", - () -> FirstPersonModelCore.instance.getConfig().renderSpinAttackModel, - b -> FirstPersonModelCore.instance.getConfig().renderSpinAttackModel = b)); + () -> cfg.renderSpinAttackModel, b -> cfg.renderSpinAttackModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.renderFlyingModel", - () -> FirstPersonModelCore.instance.getConfig().renderFlyingModel, - b -> FirstPersonModelCore.instance.getConfig().renderFlyingModel = b)); + () -> cfg.renderFlyingModel, b -> cfg.renderFlyingModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.renderSwimTransitionModel", - () -> FirstPersonModelCore.instance.getConfig().renderSwimTransitionModel, - b -> FirstPersonModelCore.instance.getConfig().renderSwimTransitionModel = b)); + () -> cfg.renderSwimTransitionModel, b -> cfg.renderSwimTransitionModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.renderScopingModel", - () -> FirstPersonModelCore.instance.getConfig().renderScopingModel, - b -> FirstPersonModelCore.instance.getConfig().renderScopingModel = b)); + () -> cfg.renderScopingModel, b -> cfg.renderScopingModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.modifyPlayerModel", - () -> FirstPersonModelCore.instance.getConfig().modifyPlayerModel, - b -> FirstPersonModelCore.instance.getConfig().modifyPlayerModel = b)); + () -> cfg.modifyPlayerModel, b -> cfg.modifyPlayerModel = b)); options.add(getOnOffOption("text.firstperson.option.firstperson.keepModelVisible", - () -> FirstPersonModelCore.instance.getConfig().keepModelVisible, - b -> FirstPersonModelCore.instance.getConfig().keepModelVisible = b)); + () -> cfg.keepModelVisible, b -> cfg.keepModelVisible = b));Optional UX: Consider disabling the offset controls when “Modify Player Model” is off, to signal they won’t have effect. I can wire that up if the widget API supports dependent enable/disable.
src/main/resources/assets/firstperson/lang/zh_cn.json (1)
31-54: 修正措辞并与实际行为对齐
- “在游泳时禁用原版手部显示（当然也会忽略动态模式）”用语不太自然；简化为“同时忽略”更好。
- “保持模型显示（动态模式除外）”与实现不符。LogicHandler 中 keepModelVisible 仅影响各状态的显示开关，不与 dynamicMode（手部逻辑）绑定。
建议如下修改：
- "text.firstperson.option.firstperson.vanillaHandsSkipSwimming.tooltip": "在游泳时禁用原版手部显示（当然也会忽略动态模式）", + "text.firstperson.option.firstperson.vanillaHandsSkipSwimming.tooltip": "在游泳时禁用原版手部显示（同时忽略动态模式）", - "text.firstperson.option.firstperson.modifyPlayerModel": "修改玩家模型偏移", + "text.firstperson.option.firstperson.modifyPlayerModel": "修改玩家模型", - "text.firstperson.option.firstperson.keepModelVisible.tooltip": "保持模型显示（动态模式除外）" + "text.firstperson.option.firstperson.keepModelVisible.tooltip": "无视各状态渲染开关保持模型显示"如 keepModelVisible 确实应与 dynamicMode 交互，请指出对应实现位置，我会调整表述。
src/main/java/dev/tr7zw/firstperson/LogicHandler.java (2)
43-48: Guard against null player before querying stateWhen no world is loaded, client.player can be null. Add a quick guard to avoid potential NPEs in the activation handler.
FirstPersonAPI.registerPlayerHandler((ActivationHandler) () -> { + if (client.player == null) { + return false; + } if (((client.player.isSleeping() && !fpm.getConfig().renderSleepingModel) || (client.player.isAutoSpinAttack() && !fpm.getConfig().renderSpinAttackModel) || (client.player.isFallFlying() && !fpm.getConfig().renderFlyingModel) || (client.player.getSwimAmount(1f) != 0 && !isCrawlingOrSwimming(client.player) && !fpm.getConfig().renderSwimTransitionModel)) && !fpm.getConfig().keepModelVisible) {If the event cannot fire without a player, feel free to skip this.
141-147: Optional: use current/lerped pitch for smoother Y offsetUsing xRotO can cause a 1-tick lag. Consider using the current/lerped X-rot for smoother motion.
Example:
- if (player.xRotO > 0 && player.isUnderWater()) { - y += 0.6f * Math.sin(Math.toRadians(player.xRotO)); + float xRot = player.getXRot(); // or Mth.lerp(delta, player.xRotO, player.getXRot()) + if (xRot > 0 && player.isUnderWater()) { + y += 0.6f * Math.sin(Math.toRadians(xRot)); } else { - y += 0.01f * -Math.sin(Math.toRadians(player.xRotO)); + y += 0.01f * -Math.sin(Math.toRadians(xRot)); }FPVersionless/src/main/java/dev/tr7zw/firstperson/versionless/config/FirstPersonSettings.java (1)
24-24: Style nit: spacing around assignmentMinor consistency nit.
- public boolean renderSwimTransitionModel= false; // 游泳过渡 + public boolean renderSwimTransitionModel = false; // 游泳过渡
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
FPVersionless/src/main/java/dev/tr7zw/firstperson/versionless/config/FirstPersonSettings.java(1 hunks)src/main/java/dev/tr7zw/firstperson/LogicHandler.java(5 hunks)src/main/java/dev/tr7zw/firstperson/config/ConfigScreenProvider.java(1 hunks)src/main/resources/assets/firstperson/lang/en_us.json(1 hunks)src/main/resources/assets/firstperson/lang/zh_cn.json(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/dev/tr7zw/firstperson/config/ConfigScreenProvider.java (1)
src/main/java/dev/tr7zw/firstperson/FirstPersonModelCore.java (1)
FirstPersonModelCore(14-100)
🔇 Additional comments (4)
src/main/resources/assets/firstperson/lang/zh_cn.json (1)
6-10: 新增分栏翻译对齐 UI 使用点，LGTMKeys align with the tabs added in ConfigScreenProvider; wording looks natural.
src/main/java/dev/tr7zw/firstperson/LogicHandler.java (2)
60-62: Scoping behavior toggle is correctThe gating matches the intended “don’t render while scoping unless allowed or forced visible”.
107-108: Early-return check adds the missing guardRespecting modifyPlayerModel here avoids unnecessary math and side effects. Good addition.
FPVersionless/src/main/java/dev/tr7zw/firstperson/versionless/config/FirstPersonSettings.java (1)
16-28: Default render toggles now hide the model—please confirm backward-compatibility intentBy defaulting all new
render*flags tofalse, existing users will no longer see the first-person model in those states. If you intend to preserve current behavior, consider:• In
FPVersionless/src/main/java/dev/tr7zw/firstperson/versionless/config/FirstPersonSettings.java:
- Change
renderSleepingModel,renderSpinAttackModel,renderFlyingModel, andrenderSwimTransitionModeltotrue.- Leave
renderScopingModelasfalse(matches existing scope behavior).
• BumpconfigVersion(currently2) to3and implement migration logic so users’ old settings carry over.Suggested diff:
--- a/FirstPersonSettings.java +++ b/FirstPersonSettings.java @@ lines 7,16 - public int configVersion = 2; + public int configVersion = 3; @@ lines 16-21 - public boolean renderSleepingModel = false; // 睡觉 - public boolean renderSpinAttackModel = false; // 三叉戟旋风斩 - public boolean renderFlyingModel = false; // 是否渲染飞行模型 - public boolean renderSwimTransitionModel = false; // 游泳过渡 - public boolean renderScopingModel = false; // 望远镜 + public boolean renderSleepingModel = true; // 睡觉 + public boolean renderSpinAttackModel = true; // 三叉戟旋风斩 + public boolean renderFlyingModel = true; // 是否渲染飞行模型 + public boolean renderSwimTransitionModel = true; // 游泳过渡 + public boolean renderScopingModel = false; // 望远镜 (保持当前隐藏行为)
Comment on lines
116
to
120
| if (player.xRotO > 0) { | ||
| bodyOffset = Constants.SWIM_UP_BODY_OFFSET; | ||
| bodyOffset = Constants.SWIM_UP_BODY_OFFSET + fpm.getConfig().swimXOffset / 100f; // 保留原值并加上配置项 | ||
| } else { | ||
| bodyOffset = Constants.SWIM_DOWN_BODY_OFFSET; | ||
| bodyOffset = Constants.SWIM_DOWN_BODY_OFFSET + fpm.getConfig().crawlXOffset / 100f; // 保留原值并加上配置项 | ||
| } |
There was a problem hiding this comment.
Bug: crawlXOffset applied based on pitch, not state (swim vs crawl)
Within isCrawlingOrSwimming, you’re choosing crawlXOffset when looking down, which will also trigger during underwater swimming. The choice of swimXOffset vs. crawlXOffset should depend on whether the player is underwater, not on pitch.
Proposed fix:
- if (player.xRotO > 0) {
- bodyOffset = Constants.SWIM_UP_BODY_OFFSET + fpm.getConfig().swimXOffset / 100f; // 保留原值并加上配置项
- } else {
- bodyOffset = Constants.SWIM_DOWN_BODY_OFFSET + fpm.getConfig().crawlXOffset / 100f; // 保留原值并加上配置项
- }
+ final boolean underwater = player.isUnderWater();
+ final float base = (player.xRotO > 0)
+ ? Constants.SWIM_UP_BODY_OFFSET
+ : Constants.SWIM_DOWN_BODY_OFFSET;
+ bodyOffset = base + (underwater
+ ? fpm.getConfig().swimXOffset / 100f
+ : fpm.getConfig().crawlXOffset / 100f);If there’s a more precise crawl detector available (e.g., Pose.SWIMMING vs. conditions), we can use that instead of isUnderWater().
📝 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 (player.xRotO > 0) { | |
| bodyOffset = Constants.SWIM_UP_BODY_OFFSET; | |
| bodyOffset = Constants.SWIM_UP_BODY_OFFSET + fpm.getConfig().swimXOffset / 100f; // 保留原值并加上配置项 | |
| } else { | |
| bodyOffset = Constants.SWIM_DOWN_BODY_OFFSET; | |
| bodyOffset = Constants.SWIM_DOWN_BODY_OFFSET + fpm.getConfig().crawlXOffset / 100f; // 保留原值并加上配置项 | |
| } | |
| // Determine whether the player is actually underwater (swimming) or on ground (crawling) | |
| final boolean underwater = player.isUnderWater(); | |
| // Choose the base offset by pitch | |
| final float base = (player.xRotO > 0) | |
| ? Constants.SWIM_UP_BODY_OFFSET | |
| : Constants.SWIM_DOWN_BODY_OFFSET; | |
| // Add the configured offset for swim vs. crawl | |
| bodyOffset = base + (underwater | |
| ? fpm.getConfig().swimXOffset / 100f | |
| : fpm.getConfig().crawlXOffset / 100f); |
🤖 Prompt for AI Agents
In src/main/java/dev/tr7zw/firstperson/LogicHandler.java around lines 116 to
120, the code currently selects crawlXOffset when player.xRotO <= 0 (looking
down), which incorrectly applies crawl offsets while underwater; change the
branch to pick swimXOffset when the player is swimming/underwater and
crawlXOffset when the player is actually crawling. Replace the pitch-based
condition with a swim/crawl state check (e.g., use isUnderWater() or
Pose.SWIMMING when available) so that bodyOffset = (swimming ?
Constants.SWIM_*_BODY_OFFSET + fpm.getConfig().swimXOffset/100f :
Constants.SWIM_*_BODY_OFFSET + fpm.getConfig().crawlXOffset/100f) choosing the
appropriate up/down constant per state and keeping the original offsets plus the
configured addition.
Closed
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.
Add configurable model options