Add some new configs #530
Add some new configs #530
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve modifications across several classes related to bullet hit particle management and status messaging in the game. A static variable for bullet hit particle multiplier was removed from the Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant Weapon
participant ModernConfigManager
participant WeaponFireAspect
Player->>Weapon: Change fire mode
alt Status messages enabled
Weapon->>Player: Send fire mode change message
end
Player->>Weapon: Adjust zoom
alt Status messages enabled
Weapon->>Player: Send zoom adjustment message
end
Player->>WeaponFireAspect: Attempt to fire
alt No ammo
WeaponFireAspect->>Player: Send no ammo message
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 2
🧹 Outside diff range and nitpick comments (5)
src/main/java/com/paneedah/weaponlib/config/ModernConfigManager.java (1)
143-145: Improve the configuration comment for clarity.While the implementation is correct, the comment could be more descriptive to better explain the purpose and impact of this setting.
Consider updating the comment to be more specific:
- @ConfigSync(category = CATEGORY_GAMEPLAY, comment = "How much particles should be spawned by a bullet.") + @ConfigSync(category = CATEGORY_GAMEPLAY, comment = "Multiplier for the number of particles spawned when a bullet hits a surface (range: 1-10).")src/main/java/com/paneedah/weaponlib/WeaponFireAspect.java (3)
213-222: LGTM! Consider fixing the indentation.The status message configuration is correctly implemented, maintaining the sound feedback as a separate concern. The indentation of the closing brace could be improved for better readability.
- } + }
Line range hint
1-654: Implementation maintains good separation of concerns.The changes effectively integrate the status message configuration while:
- Maintaining backward compatibility
- Preserving core weapon functionality
- Keeping proper error handling
- Separating visual feedback from gameplay mechanics
Missing implementation of
bulletHitParticleMultinWeaponFireAspect.java.
The configurationbulletHitParticleMultis defined and used elsewhere but is not implemented inWeaponFireAspect.javaas intended. Please ensure it is integrated correctly in this file.🔗 Analysis chain
Line range hint
8-8: Verify implementation of bulletHitParticleMult config.The PR objectives mention adding
bulletHitParticleMultconfig, but it's not utilized in this file. Please verify if this configuration should be implemented here or in another class.
shell
#!/bin/bashFind the definition of bulletHitParticleMult
rg "bulletHitParticleMult\s*="
Check usage of bulletHitParticleMult in WeaponFireAspect.java
rg "bulletHitParticleMult" src/main/java/com/paneedah/weaponlib/WeaponFireAspect.java
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for bulletHitParticleMult usage across the codebase echo "Searching for bulletHitParticleMult usage..." rg -l "bulletHitParticleMult" # Search for particle-related code in ModernConfigManager echo "Checking ModernConfigManager for particle configurations..." rg -A 5 "class ModernConfigManager"Length of output: 1058
Script:
Length of output: 98
Script:
#!/bin/bash # Locate the definition of bulletHitParticleMult rg "bulletHitParticleMult\s*=" # Find all occurrences of bulletHitParticleMult in the codebase rg "bulletHitParticleMult"Length of output: 453
src/main/java/com/paneedah/weaponlib/Weapon.java (1)
1398-1414: Consider extracting status message logic to reduce nesting.The conditional logic for sending status messages is valid but could be extracted to improve readability.
Consider extracting the status message logic into a helper method:
- if (ModernConfigManager.enableStatusMessages){ - String message; - if (result == 1) { - message = I18n.format("gui.firearmMode.semi"); - } else if (result == Integer.MAX_VALUE) { - message = I18n.format("gui.firearmMode.auto"); - } else { - message = I18n.format("gui.firearmMode.burst"); - } - LOGGER.debug("Changed fire mode of {} to {}", instance, result); - - if (instance.getPlayer() instanceof EntityPlayer) { - ((EntityPlayer) instance.getPlayer()).sendStatusMessage(new TextComponentString(I18n.format("gui.firearmMode", message)), true); - } - } + sendFireModeStatusMessage(instance, result); // Add helper method: + private void sendFireModeStatusMessage(PlayerWeaponInstance instance, int fireMode) { + if (!ModernConfigManager.enableStatusMessages || !(instance.getPlayer() instanceof EntityPlayer)) { + return; + } + + String message = fireMode == 1 ? I18n.format("gui.firearmMode.semi") + : fireMode == Integer.MAX_VALUE ? I18n.format("gui.firearmMode.auto") + : I18n.format("gui.firearmMode.burst"); + + LOGGER.debug("Changed fire mode of {} to {}", instance, fireMode); + ((EntityPlayer) instance.getPlayer()).sendStatusMessage( + new TextComponentString(I18n.format("gui.firearmMode", message)), true); + }🧰 Tools
🪛 GitHub Check: CodeScene Cloud Delta Analysis (next)
[warning] 1398-1414: ❌ New issue: Bumpy Road Ahead
changeFireMode has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/main/java/com/paneedah/mwc/MWC.java(0 hunks)src/main/java/com/paneedah/mwc/network/handlers/BlockHitMessageHandler.java(2 hunks)src/main/java/com/paneedah/weaponlib/Weapon.java(3 hunks)src/main/java/com/paneedah/weaponlib/WeaponFireAspect.java(1 hunks)src/main/java/com/paneedah/weaponlib/config/ModernConfigManager.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/paneedah/mwc/MWC.java
🧰 Additional context used
🪛 GitHub Check: CodeScene Cloud Delta Analysis (next)
src/main/java/com/paneedah/weaponlib/Weapon.java
[warning] 1398-1414: ❌ New issue: Bumpy Road Ahead
changeFireMode has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
[warning] 1481-1484: ❌ New issue: Bumpy Road Ahead
incrementZoom has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
[warning] 1509-1512: ❌ New issue: Bumpy Road Ahead
decrementZoom has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
🔇 Additional comments (2)
src/main/java/com/paneedah/mwc/network/handlers/BlockHitMessageHandler.java (1)
4-4: LGTM: Import change aligns with configuration centralization
The import change correctly reflects the move from hardcoded values to centralized configuration management.
src/main/java/com/paneedah/weaponlib/config/ModernConfigManager.java (1)
95-96: LGTM! Well-structured status messages configuration.
The new configuration option is properly implemented with clear documentation and appropriate categorization.
src/main/java/com/paneedah/mwc/network/handlers/BlockHitMessageHandler.java
Show resolved
Hide resolved
| if (ModernConfigManager.enableStatusMessages){ | ||
| if (instance.getPlayer() instanceof EntityPlayer) { | ||
| ((EntityPlayer) instance.getPlayer()).sendStatusMessage(new TextComponentString(I18n.format("gui.currentZoom", Math.round(ratio * 100))), true); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated zoom status message logic.
The status message logic is duplicated between incrementZoom and decrementZoom methods.
Extract the common logic into a helper method:
// Add helper method:
+ private void sendZoomStatusMessage(PlayerWeaponInstance instance, float ratio) {
+ if (ModernConfigManager.enableStatusMessages && instance.getPlayer() instanceof EntityPlayer) {
+ ((EntityPlayer) instance.getPlayer()).sendStatusMessage(
+ new TextComponentString(I18n.format("gui.currentZoom", Math.round(ratio * 100))), true);
+ }
+ }
// In incrementZoom:
- if (ModernConfigManager.enableStatusMessages){
- if (instance.getPlayer() instanceof EntityPlayer) {
- ((EntityPlayer) instance.getPlayer()).sendStatusMessage(new TextComponentString(I18n.format("gui.currentZoom", Math.round(ratio * 100))), true);
- }
- }
+ sendZoomStatusMessage(instance, ratio);
// In decrementZoom:
- if(ModernConfigManager.enableStatusMessages){
- if (instance.getPlayer() instanceof EntityPlayer) {
- ((EntityPlayer) instance.getPlayer()).sendStatusMessage(new TextComponentString(I18n.format("gui.currentZoom", Math.round(ratio * 100))), true);
- }
- }
+ sendZoomStatusMessage(instance, ratio);This refactoring will:
- Eliminate code duplication
- Reduce nested conditional complexity
- Make the code more maintainable
Also applies to: 1509-1512
🧰 Tools
🪛 GitHub Check: CodeScene Cloud Delta Analysis (next)
[warning] 1481-1484: ❌ New issue: Bumpy Road Ahead
incrementZoom has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
Qodana for JVM21603 new problems were found
☁️ View the detailed Qodana report Detected 18 dependenciesThird-party software listThis page lists the third-party software dependencies used in Modern Warfare Cubed
Contact Qodana teamContact us at [email protected]
|
📝 Description
Adds some new configs: bulletHitParticleMult and enableStatusMessages
Why?
🎯 Goals
Add some new configs for modpack makers (Ex: me for ValkReno)
❌ Non Goals
Break the mod
🚦 Testing
Yes, also bulletHitParticleMult cant be changed in-game
⏮️ Backwards Compatibility
1.12.2
📚 Related Issues & Documents
N/a
📖 Added to documentation?