Skip to content

Vibe codede#320

Open
LotusRPG wants to merge 2 commits into
magemonkeystudio:devfrom
LotusRPG:lotus-change-plugin
Open

Vibe codede#320
LotusRPG wants to merge 2 commits into
magemonkeystudio:devfrom
LotusRPG:lotus-change-plugin

Conversation

@LotusRPG
Copy link
Copy Markdown
Contributor

@LotusRPG LotusRPG commented May 15, 2026

COPED FROM COPILOT:

Now I'll fetch the detailed information needed for a comprehensive code review. Let me get the files changed, commits, reviews, and check runs.## Detailed Code Review

This is a major feature expansion for the Divinity plugin—a Minecraft RPG systems plugin. The PR introduces dynamic buffing systems, new penetration mechanics, updated combat formulas, and numerous quality-of-life improvements. Let me break it down by theme:


1. New Stat Systems: Damage & Defense Buffs

Files: DynamicBuffStat.java (new), PenetrationStat.java (new), configuration files

What's being added:

  • DynamicBuffStat — per-damage-type % buffs for both damage and defense (e.g., "5% Physical Damage Buff")
  • PenetrationStat — per-damage-type penetration stats, supporting both flat and % penetration modes
  • New config files auto-generated at startup:
    • damage_buffs_percent.yml
    • defense_buffs_percent.yml
    • penetration.yml

Quality: ✅ Well-structured, follows existing patterns. Both classes extend DuplicableItemLoreStat, implement proper JSON NBT serialization, and have clear Javadocs.


2. Combat Formula Refactoring

File: DamageManager.java

Changes:

  • New defense formula modes: LEGACY (old 1:1), FACTOR (Minecraft formula), and CUSTOM (configurable formula string with math evaluation)
  • Custom formula support: Placeholders like damage, defense, toughness, defense_<id> allow server admins to tune defense calculations
  • Flat penetration overflow mechanic: When flat pen exceeds total defense, can trigger bonus damage via configurable formula
  • Skill-specific critical stats: Separated CRITICAL_RATE/DAMAGE (auto-attacks) from SKILL_CRITICAL_RATE/DAMAGE (skills from Fabled)
  • Damage buff & defense buff application in the main calculation loop

Quality: ⚠️ Complex but necessary. The logic is dense (~200 lines of changes), with nested formula evaluation. The separation of skill vs. auto-attack crit is good. However, the evaluateDefenseFormula() helper using string replacement feels fragile—consider a proper expression parser for production.


3. Fabled Hook Integration (Skill API)

File: FabledHook.java

Major additions:

  • onMaxManaChange() — sync MAX_MANA item stat to Fabled mana pools
  • updateFabledAttributes() / clearFabledAttributes() — manage Fabled attribute modifiers from Divinity items
  • applyStatScale() — allows Fabled to scale Divinity stats (e.g., "skill buffs increase critical rate")
  • Metadata-based damage flags (0x01 = ignore crit, 0x04 = ignore block, etc.)
  • New event handlers: onDivinityDamageStart, onDivinityDodge

Quality: ✅ Well-implemented bi-directional integration. Metadata flag system is clever for cross-plugin communication.


4. Item Generator Enhancements

Files: ItemGeneratorManager.java, DuplicableStatGenerator.java (new), StatCategoryGUI.java (new), MMobEquipCmd.java (new)

New features:

  • DuplicableStatGenerator — rolls damage buffs, defense buffs, and penetrations independently (vs. pooling)
  • Icon material per stat — cosmetic editor feature (shows different materials for different stat categories)
  • StatCategoryGUI — intermediate menu to choose between General/Damage%/Defense%/Penetration stats
  • MMobEquipCmd — new command to equip generated items on mobs (useful for bosses). Supports UUID-based or ray-trace targeting
  • -noenchants flag for drop/give commands

Quality: ✅ Good UX progression. The category GUI bridges the gap for simpler lore editing. DuplicableStatGenerator is well-designed, though the pattern repeats logic from TypedStatGenerator.


5. Config & Engine Changes

Files: EngineCfg.java, engine.yml, plugin.yml

Additions:

  • ATTRIBUTES_HIDE_FLAGS — toggle attribute/enchant visibility on items
  • DEFENSE_FORMULA_MODE, CUSTOM_DEFENSE_FORMULA, COMBAT_OVERFLOW_PEN_* config options
  • FULL_LEGACY flag — global legacy mode that disables most new systems
  • LORE_STYLE_ENCHANTMENTS_ROMAN_SYSTEM — toggle Roman numerals vs. Arabic numbers in lore
  • New vanilla MC 1.21+ attributes (SCALE, WATER_MOVEMENT_EFFICIENCY, JUMP_STRENGTH, etc.)

Quality: ✅ Comprehensive. Good separation of concerns with legacy toggles preserving backward compatibility.


6. Durability & Mending

File: DurabilityStat.java, ItemDurabilityListener.java

Changes:

  • Improved vanilla durability bar sync — now shows a percentage of custom durability
  • New onMending() handler — blocks vanilla mending, applies custom durability logic instead
  • Safeguard: if custom durability is not max but vanilla bar would show 100%, it shows 1 damage instead

Quality: ✅ Solid fix. The previous logic had scaling issues; this is much cleaner.


7. Permissions Refactor

File: Perms.java, multiple usage sites

Changes:

  • Removed legacy permission method: Perms.has() replaced with native player.hasPermission()
  • Legacy DIVINITY constant removed (was for dual quantumrpg./divinity. namespace support)

Quality: ⚠️ Breaking change. This simplifies the codebase but removes support for the old quantumrpg.* permission namespace. Servers using that namespace will break. Consider a migration note in changelogs.


8. Branding/Attribution Updates

Files: pom.xml, multiple Java files, CustomItemsManager.java

Changes:

  • Copyright headers: © 2026 VoidEdgeCopyright 2024 MageMonkeyStudio
  • Version downgrades in pom.xml (e.g., 1.0.2-R0.661.0.2-R0.57)
  • Removed jitpack.io repository (likely for licensing reasons)

Quality: ✅ Clean. Appears to be a rebranding as the plugin moved to new maintainers.


9. EntityStats Enhancements

File: EntityStats.java

Additions:

  • getPenetration(), getDynamicBuff() methods to query new stat types
  • Fabled attribute syncing with applyStatScale()
  • Support for MC 1.21+ vanilla attributes (SCALE, BLOCK_BREAK_SPEED, JUMP_STRENGTH, FLYING_SPEED, GRAVITY, etc.)
  • Attribute modifiers use fixed UUIDs for persistence

Quality: ✅ Comprehensive. The UUID-based approach ensures modifiers can be cleanly replaced on updates.


Issues & Concerns

  1. String-based formula evaluation (DamageManager.evaluateDefenseFormula()) — uses naive string replacement. Safe for config-only strings, but could be safer with a dedicated parser.

  2. Permission breaking changePerms.has() removal is a major breaking change for servers with legacy permissions. Needs clear migration guide.

  3. Config backward compatibility — New stats (damage buffs, defense buffs, penetration) are auto-generated at startup. Servers upgrading will get defaults. This could surprise players or affect balance. Consider a migration guide.

  4. Test coverage — Some tests removed from DivinityProviderTest.java (module-based item lookups). Ensure coverage is still adequate.

  5. Null safetyFabledHook.applyStatScale() has try-catch-all. While defensive, it could hide real errors.

  6. Merge conflict risk — 64 files touched; plugin.yml especially has a massive diff (500+ lines). High risk of merge conflicts if other PRs touch permissions.


Recommendations

Approve with these notes:

  1. Add migration guide for the permission changes (quantumrpg.* → divinity.*)
  2. Document new config options in the CHANGELOG with default values and examples
  3. Consider adding a formula validation step in EngineCfg to catch config mistakes early
  4. Increase test coverage for the new stat generators (DuplicableStatGenerator, PenetrationStat)
  5. Add Javadoc for the new combat formula fields in EngineCfg

Summary

Overall: This is a high-quality, feature-rich expansion that significantly enhances the plugin's combat depth. The new stat systems (buffs, penetration) are well-implemented, the Fabled integration is solid, and the refactored item generators are user-friendly. The main risks are the breaking permission changes and the complexity of the new combat formulas, but both are well-documented in the code.

Risk Level: 🟡 Medium (breaking permission change, complex formulas) → Manageable with migration guide

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.

1 participant