Dev #144
Dev #144
Conversation
The previous 30-second timeout was insufficient for some network conditions, causing premature failures. Doubling to 60 seconds improves reliability.
Enable the feature only when explicitly selected by the user to prevent unintended gameplay effects.
WalkthroughAdds a re-enable UI action and FeatureManager.reEnable(), persists/restores enabled feature names during disableAll/reEnable, explicitly disables GodMode by default, increases two HTTP timeouts, and bumps mod version from v4.38.3-v8 to v4.39.0-v8. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as FeatureSettingDialog (UI)
participant FM as FeatureManager
participant S as SettingsStore
participant F as Feature instances
UI->>FM: click "Re-enable" -> reEnable()
FM->>S: read "mindustry-tool.enableds"
S-->>FM: list of enabled feature names
FM->>F: for each feature name -> enable()
F-->>FM: enable ack
FM-->>UI: update UI / completion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)src/mindustrytool/features/settings/FeatureSettingDialog.java (1)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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 |
Add a button to re-enable previously enabled features after disabling all. This allows users to easily restore their preferred feature state without manual reconfiguration.
The send button's text was updated but the click action was not triggered, causing the chat message to not be sent. This ensures the button behaves correctly after text localization.
![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
🤖 Fix all issues with AI agents
In `@src/mindustrytool/features/settings/FeatureSettingDialog.java`:
- Around line 134-136: Fix the typo in the translation key used in the
FeatureSettingDialog button by changing "@reeanable" to "@reenable", and after
calling FeatureManager.getInstance().reEnable() ensure the dialog UI is
refreshed: call the dialog rebuild/refresh method (or re-create the dialog
contents) so the updated feature state is shown and add a short user feedback
action (e.g., a toast/label update or call to show a confirmation) so users see
the re-enable succeeded; locate this in FeatureSettingDialog where
table.button(...) is defined and in the callback that invokes
FeatureManager.getInstance().reEnable().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mod.hjsonsrc/mindustrytool/features/FeatureManager.javasrc/mindustrytool/features/settings/FeatureSettingDialog.java
🚧 Files skipped from review as they are similar to previous changes (1)
- mod.hjson
🧰 Additional context used
🧬 Code graph analysis (1)
src/mindustrytool/features/settings/FeatureSettingDialog.java (1)
src/mindustrytool/features/FeatureManager.java (1)
FeatureManager(6-85)
🔇 Additional comments (2)
src/mindustrytool/features/FeatureManager.java (2)
17-26: Implementation looks correct.The
reEnable()method properly loads the saved feature names and re-enables matching features. TheSeq::newsupplier ensures a safe default when the setting doesn't exist.Minor note: The settings key
"mindustry-tool.enableds"uses a hyphen while other feature keys use"mindustrytool.feature.xxx.enabled"(no hyphen). Consider aligning the naming convention for consistency.[approve_code_changes, suggest_optional_refactor]
29-31: LGTM!The
disableAll()method now correctly persists the currently enabled features before disabling them, enabling thereEnable()flow to restore the previous state.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| table.button("@reeanable", () -> { | ||
| FeatureManager.getInstance().reEnable(); | ||
| }).row(); |
There was a problem hiding this comment.
Typo in translation key: @reeanable → @reenable
The translation key has an extra 'a'. Additionally, after re-enabling features, the dialog should rebuild to reflect the updated state and provide user feedback.
Proposed fix
- table.button("@reeanable", () -> {
+ table.button("@reenable", () -> {
FeatureManager.getInstance().reEnable();
+ rebuild();
+ Vars.ui.showInfoFade("@features.reenabled");
}).row();📝 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.
| table.button("@reeanable", () -> { | |
| FeatureManager.getInstance().reEnable(); | |
| }).row(); | |
| table.button("@reenable", () -> { | |
| FeatureManager.getInstance().reEnable(); | |
| rebuild(); | |
| Vars.ui.showInfoFade("@features.reenabled"); | |
| }).row(); |
🤖 Prompt for AI Agents
In `@src/mindustrytool/features/settings/FeatureSettingDialog.java` around lines
134 - 136, Fix the typo in the translation key used in the FeatureSettingDialog
button by changing "@reeanable" to "@reenable", and after calling
FeatureManager.getInstance().reEnable() ensure the dialog UI is refreshed: call
the dialog rebuild/refresh method (or re-create the dialog contents) so the
updated feature state is shown and add a short user feedback action (e.g., a
toast/label update or call to show a confirmation) so users see the re-enable
succeeded; locate this in FeatureSettingDialog where table.button(...) is
defined and in the callback that invokes
FeatureManager.getInstance().reEnable().
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mindustrytool/features/chat/global/ChatOverlay.java (1)
821-829: Remove programmatic click after send to avoid empty-content toast.
sendButton.fireClick()will re-triggersendMessage()after clearing input, producing a "@chat.empty-content" toast on every successful send and potentially executing other click handlers. Drop the click and rely on state updates; if a refresh is needed, callinvalidateHierarchy()or similar.💡 Proposed fix
if (sendButton != null) { sendButton.setText("@chat.send"); - sendButton.fireClick(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mindustrytool/features/chat/global/ChatOverlay.java
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
When the send button is clicked, it should trigger the input field's click event instead of the button's own click event to ensure proper focus behavior.
Add a search field to filter features by name. Extract rebuild logic into separate method to handle filtering. Adjust layout width calculation and card width constraints for better responsiveness.
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Chores
✏️ Tip: You can customize this high-level summary in your review settings.