MineColonies

MineColonies

59M Downloads

[BUG] Miner Lucky Ore config ignored

Ramact opened this issue ยท 7 comments

commented

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.
commented

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.

commented

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.

commented

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.

commented

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?

commented

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.

commented

AFAIK yes, per #9555.

commented

@uecasm is this fixed now?