fix: force named palettes to be unique #6417
Draft
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of change (The Why)
In mods it's pretty common to add in new locations. Sometimes they're variants of existing locations and have a modified palette. The problem is that while the location definition becomes a variant, palettes are unilaterally overwritten if they redefine an existing one and will be used as the palette for all locations that use it. Redefinition of palettes has in the past caused various loading errors that were hard to diagnose because there was a disconnect between how the error was reported and where the error actually was.
Describe the solution (The How)
Force named palettes to be unique by using
map::try_emplace( key, val )instead ofmap[key] = valand make it a fatal json loading error so that it's very obviously not allowed.Describe alternatives you've considered
Could have not made it a fatal error, or had some way to define in the palette that it's intended to be a redefinition and is not incidental.
Testing
Duplicated the palette in
./data/json/mapgen_palettes/abandoned_textile_mill.jsonto force a redefinition.Tried to load a character.
Got the expected error at the expected location and was kicked out to the main menu.
Checklist
Mandatory
closes #1234in Summary of the PR so it can be closed automatically.mainso it won't cause conflict when updatingmainbranch later.Optional
doc/folder.