Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Miner Lucky Ore config ignored #9495

Open
4 of 5 tasks
Ramact opened this issue Oct 19, 2023 · 5 comments
Open
4 of 5 tasks

[BUG] Miner Lucky Ore config ignored #9495

Ramact opened this issue Oct 19, 2023 · 5 comments
Labels
Bug Gotta squash 'em all!

Comments

@Ramact
Copy link

Ramact commented Oct 19, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Are you using the latest MineColonies Version?

  • I am running the latest alpha version of MineColonies for my Minecraft version.

Did you check on the Wiki? or ask on Discord?

  • I checked the MineColonies Wiki and made sure my issue is not covered there. Or I was sent from discord to open an issue here.

What were you playing at the time? Were you able to reproduce it in both settings?

  • Single Player
  • Multi Player

Minecraft Version

1.19.2

MineColonies Version

1.19.2-1.1.246-BETA

Structurize Version

1.19.2-1.0652-BETA

Related Mods and their Versions

No response

Current Behavior

I currently have this setup for testing purposes for the lucky ore config ["minecraft:coal_ore!2", "minecraft:copper_ore!40", "minecraft:iron_ore!48"]
luckyblockchance = 18

Despite the settings the miner will only mine coal and copper, increasing coal ore chance or decreasing makes little difference as iron ore, diamond, redstone, lapis and emerald will never appear as a lucky ore block.

This is the Fortress style level 3 miner and prior to version 1.19.2-1.1.246-BETA and minecolonies-1.19.2-1.1.241-BETA the luckyore config was working as intended

Expected Behavior

Miner should be respecting the blocks listen in the lucky ore config list.

Reproduction Steps

1.Edit Minecolonies Config file to set the luckyorechance to a higher number for testing as well as the list of available lucky ore blocks.
2.Make sure caveores mod is installed so the miner doesnt mine ore that exists in the world
3.Watch the miner mine.

Logs

https://gist.github.com/Ramact/0a08570c7c9dbf63145154646bc364ac

Anything else?

  • Add a thumbs-up to the bug report if you are also affected. This helps the bug report become more visible to the team and doesn't clutter the comments.
  • Add a comment if you have any insights or background information that isn't already part of the conversation.
@Ramact Ramact added the Bug Gotta squash 'em all! label Oct 19, 2023
@Vexikuri
Copy link

Went looking for this as well. I'm running single player 1.20.1 and have lucky block chances to 75 in the config and not getting drops.

@Nullzee
Copy link

Nullzee commented Nov 7, 2023

I'm getting the same issue, had a look and around line 817 of CompatibilityManager seems to be broken.

image

@Raycoms . buildingLevel and rarity are being set to the same thing (assuming the split array is length 3) which is breaking everything else further on. Also it's testing for a split length of 3 while the config of lucky ores will only ever be size 2, meaning that buildingLevel will only ever be 0.

These changes were made on 11/10/23 by Efinder2 when he added different luckyOre configs per miner level, which coincides around when Ramact says the configs were working previously.

@uecasm
Copy link
Contributor

uecasm commented Nov 7, 2023

While it's true that the default config won't ever produce 3 values from the split, the point of #9371 was to allow a server to change the config such that a 3-value entry will be parsed into item, rarity, and mine level, in that order.

But you're correct that the rarity was not being parsed correctly for the default 2-item case, so I've incorporated your suggestion into a fix PR. Thanks for the investigation.

@Nullzee
Copy link

Nullzee commented Nov 7, 2023

While it's true that the default config won't ever produce 3 values from the split, the point of #9371 was to allow a server to change the config such that a 3-value entry will be parsed into item, rarity, and mine level, in that order.

But you're correct that the rarity was not being parsed correctly for the default 2-item case, so I've incorporated your suggestion into a fix PR. Thanks for the investigation.

As a workaround for my world, do you think changing luckyOres config to minecraft:diamond_ore!2!5 etc would fix the config issue for a level 5 miner? Or is the rest of the implementation still not there?

@uecasm
Copy link
Contributor

uecasm commented Nov 7, 2023

Actually, I think my comment was mistaken. Looking again, it was the 3-value case that it would have messed up, and in the 2-value case it should have parsed the rarity correctly. So perhaps the true bug is elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Gotta squash 'em all!
Projects
None yet
Development

No branches or pull requests

4 participants