Skip to content

Add per-player toggle for Mining skill benefit to fix Fortune/Silk Touch conflicts - #118

Draft
dmccoystephenson with Copilot wants to merge 17 commits into
developfrom
copilot/update-mining-benefit-logic
Draft

Add per-player toggle for Mining skill benefit to fix Fortune/Silk Touch conflicts#118
dmccoystephenson with Copilot wants to merge 17 commits into
developfrom
copilot/update-mining-benefit-logic

Conversation

Copilot AI commented Oct 8, 2025

Copy link
Copy Markdown
Contributor

Problem

The Mining skill's auto-smelt benefit conflicts with Fortune and Silk Touch enchantments. When the benefit triggers (10% chance), it deletes the ore block and drops a single smelted item, which:

  • Prevents Fortune enchantments from multiplying ore drops (e.g., Fortune III on diamond ore should give 2-4 diamonds, not 1 ingot)
  • Prevents Silk Touch from preserving ore blocks for later processing
  • Makes the skill less valuable in late-game when players have enchanted tools

This is helpful early game but frustrating later when players want full control over their ore drops.

Solution

Implemented a per-player toggle system that allows players to disable skill benefits on demand using the /ss toggle <skillName> command. Players can now:

  • Disable the Mining auto-smelt when using Fortune/Silk Touch tools
  • Re-enable it when they want the convenience of auto-smelting
  • Still gain Mining experience regardless of the toggle state

Implementation Details

Core Changes:

  • Added disabledBenefits HashMap to PlayerRecord to track benefit preferences per skill
  • Modified Mining.executeReward() to check if benefit is enabled before executing (6 lines added)
  • Created ToggleCommand to handle the new /ss toggle command with proper validation
  • Updated save/load methods with backward compatibility for existing player data

User Experience:

  • Benefits are enabled by default - no behavior change for existing players
  • Toggle state persists across server restarts
  • Clear feedback messages when toggling benefits
  • Command added to help text: /ss toggle (skillName)

Usage Example

# Late game with Fortune III pickaxe - disable auto-smelt
/ss toggle Mining
> You have disabled the benefit for the Mining skill.

# Mine diamond ore with Fortune III - get 2-4 diamonds per ore (normal behavior)

# Back to convenience mode - enable auto-smelt
/ss toggle Mining
> You have enabled the benefit for the Mining skill.

Technical Notes

  • Backward Compatible: Old player save files load correctly without data migration
  • Minimal Changes: Only 5 files modified (118 insertions, 1 deletion)
  • Extensible Design: System can be applied to other skills if needed in the future
  • Permission: Requires ss.toggle permission (defaults to available for all players)

Fixes #[issue_number]

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • hub.spigotmc.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.11/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.11/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.11 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.11/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/SimpleSkills/SimpleSkills org.codehaus.plexus.classworlds.launcher.Launcher clean compile (dns block)
  • jitpack.io
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.11/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.11/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.11 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.11/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/SimpleSkills/SimpleSkills org.codehaus.plexus.classworlds.launcher.Launcher clean compile (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>Simply Skills Mining Benefit</issue_title>
<issue_description>The mining skill "automagically" smelts ore you mine on a small chance, which is nice in the early game, but not so great in the later game. Enchanted tools with fortune and silk touch apply to ores normally, but on the chance the benefit happens, it will delete the ore and output one smelted item correlated to the ore. The is does not allow the player to silk touch the ore to receive the block, or to fortune the ore to get more drops.

The idea would be to change the benefit or to allow the benefit to be toggled per skill per player.</issue_description>

Comments on the Issue (you are @copilot in this section)

@dmccoystephenson This is some really helpful feedback thank you guys for your comments!
Fixes #112

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 4 commits September 21, 2025 16:06
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Copilot AI changed the title [WIP] Update mining skill to allow toggling benefits for players Add per-player toggle for Mining skill benefit to fix Fortune/Silk Touch conflicts Oct 8, 2025
Copilot AI requested a review from dmccoystephenson October 8, 2025 05:09
dmccoystephenson and others added 12 commits January 5, 2026 19:40
…e() usage, and handle() active guard

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…thub.com:Dans-Plugins/SimpleSkills into copilot/fix-751a9037-3a32-417b-8065-948a85ffe2bd
… totals

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…17b-8065-948a85ffe2bd

Fix skill activation/deactivation persistence across server restarts
Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Dans-Plugins/SimpleSkills/sessions/30bcb888-b3bd-40e1-896a-97e301aa5966
…g paths, Ponder install step, standardize repo links

Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Dans-Plugins/SimpleSkills/sessions/fbdfd48c-b863-4ded-bc3e-9825f2fc9cca
…-medieval-factions

Align documentation with Medieval Factions plugin conventions
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.

2 participants