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

Treasure map generation don't respect default setting #9563

Closed
patokeni opened this issue Aug 2, 2023 · 1 comment · Fixed by #9572
Closed

Treasure map generation don't respect default setting #9563

patokeni opened this issue Aug 2, 2023 · 1 comment · Fixed by #9572
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to.

Comments

@patokeni
Copy link

patokeni commented Aug 2, 2023

Expected behavior

Treasure maps should always be filled with the location of a nearby treasure regardless of whether it has been located by another map or not, which I believe is the vanilla behavior and default setting.

Observed/Actual behavior

After first treasure map was generated, subsequent treasure maps become effectively empty maps and only a name left.

Steps/models to reproduce

  1. Create a new world.
  2. Find several shipwreck and get the treasure maps.
    If we just create a new world, everything seems to work just fine, except every time a map gets generated, a new treasure is located. But if we explored all the treasure around a specific area, when a new map around here is needed to be generated, the server would struggle to find a new treasure, which I think caused a empty map to be generated (and cause lag).

Plugin and Datapack List

Not related.

Paper version

This server is running Paper version git-Paper-100 (MC: 1.20.1) (Implementing API version 1.20.1-R0.1-SNAPSHOT) (Git: f402f89)

Other

Inside ExplorationMapFunction.java we could see !serverLevel.paperConfig().environment.treasureMaps.findAlreadyDiscoveredLootTable.or(!this.skipKnownStructures) is replacing the vanilla this.skipKnownStructures. From vanilla loot tables we could know this.skipKnownStructures should be false. But the changed code is resulting a true by default whatever the loot table specify. Look into it deeper, it seems that the BooleanOrDefault class did not respect the default value. I don't know whether this is intended or not.

@patokeni patokeni added status: needs triage type: bug Something doesn't work as it was intended to. labels Aug 2, 2023
@papermc-projects papermc-projects bot moved this to 🕑 Needs Triage in Issues: Bugs Aug 2, 2023
@Machine-Maker Machine-Maker added status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. and removed status: needs triage labels Aug 4, 2023
@papermc-projects papermc-projects bot moved this from 🕑 Needs Triage to ✅ Accepted in Issues: Bugs Aug 4, 2023
@Machine-Maker
Copy link
Member

Yep, your analysis is spot on. Looks like somehow the fallback value in BooleanOrDefault was just completely ignored. Will be fixed with #9572

@github-project-automation github-project-automation bot moved this from ✅ Accepted to Done in Issues: Bugs Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepted Disputed bug is accepted as valid or Feature accepted as desired to be added. type: bug Something doesn't work as it was intended to.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants