Map accuracy & consistency pass: nation names, spawn positions, and flags across 8 maps#3780
Open
evadua wants to merge 35 commits intoopenfrontio:mainfrom
Open
Map accuracy & consistency pass: nation names, spawn positions, and flags across 8 maps#3780evadua wants to merge 35 commits intoopenfrontio:mainfrom
evadua wants to merge 35 commits intoopenfrontio:mainfrom
Conversation
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughStandardizes and renames many nation entries, updates numerous coordinates, removes several fantasy/legacy entries, and adds new countries/regions across multiple map JSON assets and resource manifests for world, continental, and special maps. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
map-generator/assets/maps/southamerica/info.json (1)
1-155:⚠️ Potential issue | 🔴 CriticalAll 8 maps have nation count mismatches between
info.jsonandmanifest.json— update both files to keep them in sync.
tests/MapConsistency.test.tschecks thatinfo.jsonandmanifest.jsonhave matching nation counts, names, and coordinates. The PR updatesinfo.jsonfor 8 maps but leaves theirmanifest.jsonfiles unchanged. All 8 will fail the test:
- southamerica: 25 nations in info, 24 in manifest
- world: 72 vs 61
- europe: 52 vs 50
- europeclassic: 37 vs 31
- oceania: 22 vs 32
- northamerica: 61 vs 49
- gatewaytotheatlantic: 27 vs 30
- giantworldmap: 107 vs 97
Update
resources/maps/<map>/manifest.jsonfor each of these 8 maps to match theirinfo.jsonchanges.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/southamerica/info.json` around lines 1 - 155, The info.json for eight maps (southamerica, world, europe, europeclassic, oceania, northamerica, gatewaytotheatlantic, giantworldmap) was changed but the corresponding resources/maps/<map>/manifest.json files were not updated, causing nation count/name/coordinate mismatches detected by tests/MapConsistency.test.ts; open each listed manifest.json and update its "nations" entries so the count, nation names, and coordinates exactly match the corresponding info.json (preserve ordering or adjust tests expectations if ordering matters), then run the MapConsistency test to confirm parity between info.json and manifest.json for each map.
🧹 Nitpick comments (5)
map-generator/assets/maps/northamerica/info.json (1)
268-368: New Canadian provinces/territories use a different flag-slug style than the U.S. states — pick one.The added Canadian entries use one-word lower-case slugs with no separator:
northwestterritories,britishcolumbia,alberta,saskatchewan,manitoba,ontario,newbrunswick,novascotia,newfoundlandandlabrador.Earlier U.S. state entries in the same file use TitleCase with underscores for multi-word names:
New_Mexico,New_York,North_Dakota,South_Carolina,South_Dakota. And the newYukon,Nunavut,Quebecentries are TitleCase (no separator).Three different conventions in one file is hard to maintain. Pick one slug style (e.g., TitleCase with underscores:
British_Columbia,Northwest_Territories,Newfoundland_and_Labrador) and rename both the entries here and the correspondingresources/flags/*.svgfiles. As per the existing learning (VariableVince, PR 957),flagmust match the SVG filename exactly, so any rename has to happen in lockstep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/northamerica/info.json` around lines 268 - 368, The file mixes flag slug styles; standardize to TitleCase with underscores (e.g., change "northwestterritories" -> "Northwest_Territories", "britishcolumbia" -> "British_Columbia", "newfoundlandandlabrador" -> "Newfoundland_and_Labrador", etc.) and update every corresponding SVG in resources/flags to the exact same filename (flag slug must match SVG filename exactly); ensure other Canadian entries like "Yukon", "Nunavut", and "Quebec" follow the same TitleCase/underscore convention so all "flag" values in the JSON (e.g., entries for Northwest Territories, British Columbia, Alberta, Saskatchewan, Manitoba, Ontario, New Brunswick, Nova Scotia, Newfoundland and Labrador, Yukon, Nunavut) and their SVG files are renamed in lockstep.map-generator/assets/maps/gatewaytotheatlantic/info.json (1)
238-246:Holy Roman Empireflag identifier mixes spaces and Title-Case while the rest of the map uses lowercase_underscore.Within this file the new historical entities use lowercase-underscore slugs (
kingdom_of_portugal,marinid,zayyanid,majorca,hafsid,navarre), butHoly Roman Empireuses spaces and Title-Case. Both styles work at runtime as long as the SVG filename matches exactly, but the inconsistency is jarring in a single file. Renaming toholy_roman_empire(and the SVG to match) would keep the convention uniform.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/gatewaytotheatlantic/info.json` around lines 238 - 246, The "flag" value "Holy Roman Empire" in the map info entry should be renamed to the lowercase_underscore convention (e.g., holy_roman_empire) to match the rest of the file (compare existing slugs like kingdom_of_portugal, marinid, zayyanid); update the corresponding SVG asset filename to exactly match the new flag slug so runtime lookup still finds the file, and leave the other fields (name, coordinates, strength) unchanged.map-generator/assets/maps/giantworldmap/info.json (1)
562-645: New subnational entries inherit the same multi-style flag-slug problem asnorthamerica/info.json.This file mixes three slug conventions in the new additions:
- TitleCase no-separator:
Washington,Kansas,Mississippi,Massachusetts,Minnesota,Michigan,Montana,Utah,Texas,California,Quebec,Alaska,Yukon,Nunavut,Siberia.- lowercase no-separator:
britishcolumbia,alberta,saskatchewan,manitoba,northwestterritories,newfoundlandandlabrador,amazonas.- Lowercase ISO codes for sovereign states:
bo,ga.If you address the slug-style consistency point in
northamerica/info.json, please apply the same change here so the same Canadian provinces/U.S. states use identical flag identifiers across the two maps. Otherwise the same SVG asset has to be committed twice under two filenames.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/giantworldmap/info.json` around lines 562 - 645, The new entries mix three flag-slug conventions (TitleCase no-separator like "Washington"/flag:"Washington", lowercase no-separator like "britishcolumbia"/flag:"britishcolumbia", and ISO codes like "bo"/flag:"bo"); update the "flag" values for the listed objects (e.g., entries with "name": "Washington", "Kansas", "Mississippi", "Massachusetts", "Minnesota", "Michigan", "Montana", "British Columbia", "Alberta", "Saskatchewan", "Manitoba", "Northwest Territories", and the sovereigns "Bolivia"/flag:"bo", "Gabon"/flag:"ga") so they follow the exact slug convention used in northamerica/info.json (make Canadian provinces and US states use the identical flag identifiers there), and normalize all flag keys to that single convention across this file to avoid duplicate SVG assets.map-generator/assets/maps/world/info.json (1)
298-303:Siberiareusingru(Russian flag) is acceptable but worth a sanity check.Two entries (
Russialine 156 andSiberialine 300) both render the Russian flag. That is a reasonable choice for a region inside Russia, but it does mean two adjacent nations on the world map will appear visually identical in the player list. If a distinct sub-flag (e.g., a Siberian regional banner) is preferred, this is the place to add it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/world/info.json` around lines 298 - 303, The "Siberia" map entry currently reuses the Russian flag ("flag": "ru") just like the existing "Russia" entry, so either confirm that duplication is intentional or replace the Siberia flag value with a distinct flag id (e.g., "siberia" or another regional code) and add the corresponding flag asset; update the "Siberia" object (coordinates/name/flag) to the chosen flag id and ensure the new flag asset is available wherever map flags are loaded so the map and player list render uniquely.map-generator/assets/maps/europe/info.json (1)
88-92: RenamingCzech Republic→Czechiais fine, but note the inconsistency witheuropeclassic/info.json.
europeclassic/info.json(line 96) still usesCzech Republic. If the goal is repo-wide consistency, please align both maps. If the two maps are intentionally styled differently, ignore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/europe/info.json` around lines 88 - 92, Choose a canonical country name and make both map info files use it; since this diff uses "name": "Czechia", update the other map's info JSON that still has "name": "Czech Republic" to "Czechia" (or vice versa if you prefer the longer form), ensuring the "name" property for that country is identical across both maps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@map-generator/assets/maps/europeclassic/info.json`:
- Around line 119-122: Update the nation name entries in the three manifest.json
files so the nation key uses the exact Unicode name "Türkiye" (not "Turkey"):
change the "Turkey" entries in the world and europeclassic manifest.json files
to "Türkiye", and add/update the missing europe manifest.json entry to "Türkiye"
to match its info.json; ensure exact string equality so MapConsistency and
GameView.ts cosmetics lookup succeed.
In `@map-generator/assets/maps/gatewaytotheatlantic/info.json`:
- Around line 175-183: The "Crown of Aragon" entry is using the same "catalonia"
flag asset, which is historically incorrect; create a new flag asset (e.g.,
aragon or crown_of_aragon SVG) and replace the "flag": "catalonia" value for the
"Crown of Aragon" map entry with that new key (refer to the "Crown of Aragon"
object and its "flag" field in the JSON), ensuring the new asset is added to the
project's flag assets and referenced by the same key.
- Around line 202-210: The City of Avignon entry currently uses the "va" flag
which is anachronistic; update the map entry (the object with "name": "City of
Avignon" and coordinates [1933, 614]) to use a new flag key such as "avignon" or
"papal-keys" instead of "va", and add the corresponding SVG asset to the flags
set (or reuse an existing generic papal-keys banner SVG) so the map references a
historically appropriate banner rather than the modern Vatican "va" flag.
In `@map-generator/assets/maps/southamerica/info.json`:
- Around line 89-92: Update the JSON entry where "name": "French Guyana" (the
object with "coordinates": [1204, 615], "strength": 1, "flag": "gf") to correct
the spelling to "French Guiana"; ensure only the name string is changed and the
rest of the object (coordinates, strength, flag) remains unchanged.
- Around line 130-153: The entries for the Brazilian states use inconsistent
casing for the flag field and drop diacritics from display names; update each
object so "name" uses the correct accented display name (e.g., "Pará", "São
Paulo") and "flag" uses a consistent lower-case slug (e.g., "para", "sao_paulo"
or whichever project slug convention is standard), ensuring the slug exactly
matches the SVG filename; modify the four entries (objects with name "Amazonas",
"Para", "Bahia", "Sao Paulo") to use the normalized "name" and "flag" values and
then verify that matching SVG files exist under resources/flags/ with those
exact flag names.
- Line 7: The JSON field "strength" is dead data because the Nation interface in
TerrainMapLoader.ts (interface Nation) and its consumers (e.g.,
NationCreation.ts) don't declare or use it; either remove the "strength" entries
from the map JSON files or add a numeric "strength" property to the Nation
interface and update all consumers that instantiate/read Nation
(TerrainMapLoader.ts, NationCreation.ts, and any code that constructs or
serializes nations) to accept and use this property (including parsing it when
loading maps and incorporating it into creation/initialization logic).
---
Outside diff comments:
In `@map-generator/assets/maps/southamerica/info.json`:
- Around line 1-155: The info.json for eight maps (southamerica, world, europe,
europeclassic, oceania, northamerica, gatewaytotheatlantic, giantworldmap) was
changed but the corresponding resources/maps/<map>/manifest.json files were not
updated, causing nation count/name/coordinate mismatches detected by
tests/MapConsistency.test.ts; open each listed manifest.json and update its
"nations" entries so the count, nation names, and coordinates exactly match the
corresponding info.json (preserve ordering or adjust tests expectations if
ordering matters), then run the MapConsistency test to confirm parity between
info.json and manifest.json for each map.
---
Nitpick comments:
In `@map-generator/assets/maps/europe/info.json`:
- Around line 88-92: Choose a canonical country name and make both map info
files use it; since this diff uses "name": "Czechia", update the other map's
info JSON that still has "name": "Czech Republic" to "Czechia" (or vice versa if
you prefer the longer form), ensuring the "name" property for that country is
identical across both maps.
In `@map-generator/assets/maps/gatewaytotheatlantic/info.json`:
- Around line 238-246: The "flag" value "Holy Roman Empire" in the map info
entry should be renamed to the lowercase_underscore convention (e.g.,
holy_roman_empire) to match the rest of the file (compare existing slugs like
kingdom_of_portugal, marinid, zayyanid); update the corresponding SVG asset
filename to exactly match the new flag slug so runtime lookup still finds the
file, and leave the other fields (name, coordinates, strength) unchanged.
In `@map-generator/assets/maps/giantworldmap/info.json`:
- Around line 562-645: The new entries mix three flag-slug conventions
(TitleCase no-separator like "Washington"/flag:"Washington", lowercase
no-separator like "britishcolumbia"/flag:"britishcolumbia", and ISO codes like
"bo"/flag:"bo"); update the "flag" values for the listed objects (e.g., entries
with "name": "Washington", "Kansas", "Mississippi", "Massachusetts",
"Minnesota", "Michigan", "Montana", "British Columbia", "Alberta",
"Saskatchewan", "Manitoba", "Northwest Territories", and the sovereigns
"Bolivia"/flag:"bo", "Gabon"/flag:"ga") so they follow the exact slug convention
used in northamerica/info.json (make Canadian provinces and US states use the
identical flag identifiers there), and normalize all flag keys to that single
convention across this file to avoid duplicate SVG assets.
In `@map-generator/assets/maps/northamerica/info.json`:
- Around line 268-368: The file mixes flag slug styles; standardize to TitleCase
with underscores (e.g., change "northwestterritories" ->
"Northwest_Territories", "britishcolumbia" -> "British_Columbia",
"newfoundlandandlabrador" -> "Newfoundland_and_Labrador", etc.) and update every
corresponding SVG in resources/flags to the exact same filename (flag slug must
match SVG filename exactly); ensure other Canadian entries like "Yukon",
"Nunavut", and "Quebec" follow the same TitleCase/underscore convention so all
"flag" values in the JSON (e.g., entries for Northwest Territories, British
Columbia, Alberta, Saskatchewan, Manitoba, Ontario, New Brunswick, Nova Scotia,
Newfoundland and Labrador, Yukon, Nunavut) and their SVG files are renamed in
lockstep.
In `@map-generator/assets/maps/world/info.json`:
- Around line 298-303: The "Siberia" map entry currently reuses the Russian flag
("flag": "ru") just like the existing "Russia" entry, so either confirm that
duplication is intentional or replace the Siberia flag value with a distinct
flag id (e.g., "siberia" or another regional code) and add the corresponding
flag asset; update the "Siberia" object (coordinates/name/flag) to the chosen
flag id and ensure the new flag asset is available wherever map flags are loaded
so the map and player list render uniquely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 060ad1aa-3be5-4f79-b7f9-1505041878ae
⛔ Files ignored due to path filters (6)
resources/flags/hafsid.svgis excluded by!**/*.svgresources/flags/kingdom_of_portugal.svgis excluded by!**/*.svgresources/flags/majorca.svgis excluded by!**/*.svgresources/flags/marinid.svgis excluded by!**/*.svgresources/flags/navarre.svgis excluded by!**/*.svgresources/flags/zayyanid.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
map-generator/assets/maps/europe/info.jsonmap-generator/assets/maps/europeclassic/info.jsonmap-generator/assets/maps/gatewaytotheatlantic/info.jsonmap-generator/assets/maps/giantworldmap/info.jsonmap-generator/assets/maps/northamerica/info.jsonmap-generator/assets/maps/oceania/info.jsonmap-generator/assets/maps/southamerica/info.jsonmap-generator/assets/maps/world/info.json
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
map-generator/assets/maps/southamerica/info.json (2)
121-121:⚠️ Potential issue | 🟡 MinorSpelling: "French Guyana" → "French Guiana".
The standard English name for the French overseas department is "French Guiana" (the neighbouring country at lines 129 is "Guyana"). The PR description itself uses "French Guiana". This was raised earlier and marked addressed, but the file still shows "French Guyana".
📝 Suggested fix
- "name": "French Guyana", + "name": "French Guiana",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/southamerica/info.json` at line 121, Replace the incorrect value for the JSON "name" field currently set to "French Guyana" with the correct English name "French Guiana" in the map metadata (look for the "name": "French Guyana" entry in the southamerica info.json and update that string to "French Guiana").
172-203:⚠️ Potential issue | 🟡 MinorBrazilian state
flagvalues still mix three casing styles.Names now carry the correct accents (great), but the
flagvalues still mix lower-case (amazonas,bahia), PascalCase (Para), and PascalCase with a space (Sao Paulo). Pick one slug style and make the SVG filename match exactly — otherwise the flag will not resolve at runtime for those entries.📝 Suggested normalization (lower-case slugs)
"name": "Pará", - "flag": "Para" + "flag": "para" }, { "coordinates": [ 1550, 1066 ], "name": "Bahia", "flag": "bahia" }, { "coordinates": [ 1374, 1404 ], "name": "São Paulo", - "flag": "Sao Paulo" + "flag": "sao_paulo" }Whatever style you pick, please confirm the matching SVGs exist under
resources/flags/. Based on learnings from VariableVince (PR 957/959), the flag value must equal the SVG filename exactly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/southamerica/info.json` around lines 172 - 203, The flag fields for the Brazil entries are inconsistent (amazonas, Para, bahia, Sao Paulo) causing runtime lookup failures; update the "flag" values in the map info entries for the states Amazonas, Pará, Bahia, and São Paulo to a consistent slug style (e.g., lower-case, no spaces/hyphenate if preferred — e.g., "amazonas", "para", "bahia", "sao-paulo" or "sao_paulo"), then ensure the corresponding SVG filenames in resources/flags/ exactly match the chosen slugs (filename casing and spacing must be identical to the "flag" value).
🧹 Nitpick comments (1)
map-generator/assets/maps/northamerica/info.json (1)
357-450: Canadian province flag names are correctly mapped to SVGs, but casing is inconsistent and fragile for future edits.All entries have matching SVG files:
Nunavut/Yukon→ TitleCasenorthwestterritories,britishcolumbia,alberta,saskatchewan,manitoba,ontario,newbrunswick,novascotia,newfoundlandandlabrador→ all-lowercase concatenatedThe code works now, but mixing styles in the same file increases the risk of typos in future edits. Since each
flagvalue must match the SVG filename character-for-character, pick one naming convention for the Canadian set and apply it consistently throughout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/northamerica/info.json` around lines 357 - 450, The JSON uses inconsistent casing for the Canadian "flag" values which must match SVG filenames exactly; normalize all Canadian flags to a single convention (e.g., lowercase concatenated) and update the entries where they differ — specifically change "Nunavut" and "Yukon" to "nunavut" and "yukon" so they match the existing lowercase filenames like "northwestterritories", "britishcolumbia", "alberta", "saskatchewan", "manitoba", "ontario", "newbrunswick", "novascotia", and "newfoundlandandlabrador"; ensure any future additions follow this chosen convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@map-generator/assets/maps/gatewaytotheatlantic/info.json`:
- Around line 196-203: The "flag" value "Franks" in the JSON object
(coordinates/name/flag) is anachronistic and inconsistent; change the "flag"
value to a historically appropriate and lowercase key such as "france_medieval",
"fleur_de_lis", or "kingdom_of_france" (pick the one matching the actual SVG
asset), and ensure the corresponding SVG asset file is renamed to exactly match
that new key so the reference resolves correctly; update only the "flag" string
in this JSON entry and the SVG filename to keep naming consistent with other
entries like "navarre" and "majorca".
---
Duplicate comments:
In `@map-generator/assets/maps/southamerica/info.json`:
- Line 121: Replace the incorrect value for the JSON "name" field currently set
to "French Guyana" with the correct English name "French Guiana" in the map
metadata (look for the "name": "French Guyana" entry in the southamerica
info.json and update that string to "French Guiana").
- Around line 172-203: The flag fields for the Brazil entries are inconsistent
(amazonas, Para, bahia, Sao Paulo) causing runtime lookup failures; update the
"flag" values in the map info entries for the states Amazonas, Pará, Bahia, and
São Paulo to a consistent slug style (e.g., lower-case, no spaces/hyphenate if
preferred — e.g., "amazonas", "para", "bahia", "sao-paulo" or "sao_paulo"), then
ensure the corresponding SVG filenames in resources/flags/ exactly match the
chosen slugs (filename casing and spacing must be identical to the "flag"
value).
---
Nitpick comments:
In `@map-generator/assets/maps/northamerica/info.json`:
- Around line 357-450: The JSON uses inconsistent casing for the Canadian "flag"
values which must match SVG filenames exactly; normalize all Canadian flags to a
single convention (e.g., lowercase concatenated) and update the entries where
they differ — specifically change "Nunavut" and "Yukon" to "nunavut" and "yukon"
so they match the existing lowercase filenames like "northwestterritories",
"britishcolumbia", "alberta", "saskatchewan", "manitoba", "ontario",
"newbrunswick", "novascotia", and "newfoundlandandlabrador"; ensure any future
additions follow this chosen convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 32e57045-8b7b-48bd-a16f-a92ab5bfbea5
📒 Files selected for processing (8)
map-generator/assets/maps/europe/info.jsonmap-generator/assets/maps/europeclassic/info.jsonmap-generator/assets/maps/gatewaytotheatlantic/info.jsonmap-generator/assets/maps/giantworldmap/info.jsonmap-generator/assets/maps/northamerica/info.jsonmap-generator/assets/maps/oceania/info.jsonmap-generator/assets/maps/southamerica/info.jsonmap-generator/assets/maps/world/info.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
resources/maps/giantworldmap/manifest.json (1)
18-874: Rest of the manifest looks good.
- All coordinates stay inside the map bounds (
width: 4108,height: 1948).- Name standardizations match the PR description (e.g.,
USA → United States,Sapmi → Sápmi, SSR/legacy-state cleanups, singleYemen/Vietnam/Myanmarentries).- New additions (Bolivia, Gabon, Washington, Kansas, Mississippi, Montana, Massachusetts, Minnesota, Michigan, plus the Canadian provinces) sit in geographically sensible spots relative to neighboring entries.
- Reformatting
coordinatesto multi-line is purely cosmetic and consistent across the file.Minor nit (non-blocking): the new entries are appended at the end of
nations[], so the loose regional grouping in the file (Europe → Middle East → Asia → Oceania → Africa → Americas → Canada) is now slightly broken. If you want, you can move each new entry next to its regional neighbors in a follow-up to keep diffs friendly for future reviewers — totally optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/maps/giantworldmap/manifest.json` around lines 18 - 874, The nations array in manifest.json has new entries appended to the end which breaks the original loose regional grouping; please reorder the newly added nation objects (e.g., "Bolivia", "Gabon", "Washington", "Kansas", "Mississippi", "Montana", "Massachusetts", "Minnesota", "Michigan", and the Canadian provinces like "British Columbia", "Alberta", "Saskatchewan", "Manitoba", "Northwest Territories", plus any others added) so each is placed next to its regional neighbors (Americas next to other Americas, Canada provinces with Canada, African entries near Africa, US states near United States) within the "nations" array to preserve regional grouping and keep future diffs/locality friendly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/maps/europe/manifest.json`:
- Around line 339-346: The Kazakhstan entry (flag "kz", name "Kazakhstan") has
coordinates [2904, 857] where x==2904 is outside the valid range (0..width-1)
causing isValidCoord(2904, 857) to fail and also to scale out-of-bounds on the
compact map; update the coordinates[0] to a valid value (e.g., 2890) so x <=
mapWidth-1 (and floor(mapWidth/2)-1 for compact maps) to avoid relying on the
50-attempt fallback spawn logic.
In `@resources/maps/gatewaytotheatlantic/manifest.json`:
- Around line 211-218: The flag entry in manifest.json uses the capitalized key
"Franks" (and similarly "Holy Roman Empire") which is inconsistent with the
lowercase, snake_case keys used elsewhere and semantically mismatched for
14th–15th c. France; update the "flag" value to a lowercase, era-appropriate key
such as "kingdom_of_france" (or "france_medieval"), rename the corresponding SVG
asset (e.g., Franks.svg → kingdom_of_france.svg), and ensure the same lowercase
key is used wherever "Franks" (and "Holy Roman Empire") appear so the manifest's
"flag" values and asset filenames match the new snake_case naming convention.
- Around line 171-178: The manifest entry for the object with "name": "Crown of
Aragon" currently reuses "flag": "catalonia"; create a distinct flag asset
(e.g., name it "aragon" or "crown_of_aragon" and add the corresponding SVG into
your flags assets) and update the "flag" value on the "Crown of Aragon" object
in manifest.json to that new key so the map references the new Aragon banner
instead of "catalonia".
---
Nitpick comments:
In `@resources/maps/giantworldmap/manifest.json`:
- Around line 18-874: The nations array in manifest.json has new entries
appended to the end which breaks the original loose regional grouping; please
reorder the newly added nation objects (e.g., "Bolivia", "Gabon", "Washington",
"Kansas", "Mississippi", "Montana", "Massachusetts", "Minnesota", "Michigan",
and the Canadian provinces like "British Columbia", "Alberta", "Saskatchewan",
"Manitoba", "Northwest Territories", plus any others added) so each is placed
next to its regional neighbors (Americas next to other Americas, Canada
provinces with Canada, African entries near Africa, US states near United
States) within the "nations" array to preserve regional grouping and keep future
diffs/locality friendly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8259e7ab-d582-4163-b22f-a2940b48e84c
⛔ Files ignored due to path filters (1)
resources/flags/avignon.svgis excluded by!**/*.svg
📒 Files selected for processing (12)
map-generator/assets/maps/europeclassic/info.jsonmap-generator/assets/maps/gatewaytotheatlantic/info.jsonmap-generator/assets/maps/southamerica/info.jsonmap-generator/assets/maps/world/info.jsonresources/maps/europe/manifest.jsonresources/maps/europeclassic/manifest.jsonresources/maps/gatewaytotheatlantic/manifest.jsonresources/maps/giantworldmap/manifest.jsonresources/maps/northamerica/manifest.jsonresources/maps/oceania/manifest.jsonresources/maps/southamerica/manifest.jsonresources/maps/world/manifest.json
🚧 Files skipped from review as they are similar to previous changes (2)
- map-generator/assets/maps/world/info.json
- map-generator/assets/maps/europeclassic/info.json
Contributor
|
Hi @evadua, looks like prettier is failing, please run npx prettier --write ./ |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
map-generator/assets/maps/europe/info.json (1)
3-264: Optional: reduce future drift betweeninfo.jsonandmanifest.json.These files mirror the same nation dataset. Consider generating one from the other (or validating in CI) to avoid mismatch regressions later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@map-generator/assets/maps/europe/info.json` around lines 3 - 264, The info.json "nations" array is duplicated in manifest.json causing drift; add a single-source-of-truth workflow by either (A) creating a small generator script (e.g., generateNations() / scripts/generate_nations) that reads one file and emits the other (ensure it consumes the "nations" array and preserves fields name/coordinates/flag), or (B) adding a CI validation step (e.g., validate_nations() / scripts/validate_nations) that loads both info.json and manifest.json and fails the build if the "nations" arrays differ; update README and npm scripts to run the generator or validator so future edits only touch the canonical file.resources/maps/giantworldmap/manifest.json (1)
18-554: Consider adding a lightweight CI validator for map manifests.A small read-only manifest check (flags exist, unique nation names, coordinate bounds) would prevent future data regressions across map-only PRs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/maps/giantworldmap/manifest.json` around lines 18 - 554, Add a small read-only CI validator that parses the "nations" array in resources/maps/giantworldmap/manifest.json and fails the job on invalid data: check each object has non-empty "flag" and "name" fields, ensure all "name" values are unique, and verify "coordinates" are numeric pairs within the map bounds (image width/height configurable or read from a companion metadata value); implement as a simple script (e.g., scripts/validate-map-manifest.{js,py}) that returns non-zero on any error and wire it into the CI pipeline so map-only PRs run the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@map-generator/assets/maps/northamerica/info.json`:
- Around line 269-283: The JSON list of Canadian provinces/territories is
missing "Prince Edward Island"; add an object for Prince Edward Island to the
array alongside "New Brunswick", "Nova Scotia", and "Newfoundland and Labrador"
with appropriate coordinates and a flag key (e.g., "Prince Edward Island",
coordinates [x, y], "flag": "princeedwardisland") so the file contains all 13
provinces/territories; ensure the new entry follows the same object shape and
comma placement as the neighboring entries.
---
Nitpick comments:
In `@map-generator/assets/maps/europe/info.json`:
- Around line 3-264: The info.json "nations" array is duplicated in
manifest.json causing drift; add a single-source-of-truth workflow by either (A)
creating a small generator script (e.g., generateNations() /
scripts/generate_nations) that reads one file and emits the other (ensure it
consumes the "nations" array and preserves fields name/coordinates/flag), or (B)
adding a CI validation step (e.g., validate_nations() /
scripts/validate_nations) that loads both info.json and manifest.json and fails
the build if the "nations" arrays differ; update README and npm scripts to run
the generator or validator so future edits only touch the canonical file.
In `@resources/maps/giantworldmap/manifest.json`:
- Around line 18-554: Add a small read-only CI validator that parses the
"nations" array in resources/maps/giantworldmap/manifest.json and fails the job
on invalid data: check each object has non-empty "flag" and "name" fields,
ensure all "name" values are unique, and verify "coordinates" are numeric pairs
within the map bounds (image width/height configurable or read from a companion
metadata value); implement as a simple script (e.g.,
scripts/validate-map-manifest.{js,py}) that returns non-zero on any error and
wire it into the CI pipeline so map-only PRs run the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 012a0d05-7b6c-4600-ab55-7615dba7b377
📒 Files selected for processing (16)
map-generator/assets/maps/europe/info.jsonmap-generator/assets/maps/europeclassic/info.jsonmap-generator/assets/maps/gatewaytotheatlantic/info.jsonmap-generator/assets/maps/giantworldmap/info.jsonmap-generator/assets/maps/northamerica/info.jsonmap-generator/assets/maps/oceania/info.jsonmap-generator/assets/maps/southamerica/info.jsonmap-generator/assets/maps/world/info.jsonresources/maps/europe/manifest.jsonresources/maps/europeclassic/manifest.jsonresources/maps/gatewaytotheatlantic/manifest.jsonresources/maps/giantworldmap/manifest.jsonresources/maps/northamerica/manifest.jsonresources/maps/oceania/manifest.jsonresources/maps/southamerica/manifest.jsonresources/maps/world/manifest.json
✅ Files skipped from review due to trivial changes (1)
- map-generator/assets/maps/oceania/info.json
🚧 Files skipped from review as they are similar to previous changes (6)
- map-generator/assets/maps/southamerica/info.json
- map-generator/assets/maps/giantworldmap/info.json
- resources/maps/europeclassic/manifest.json
- map-generator/assets/maps/world/info.json
- resources/maps/world/manifest.json
- resources/maps/northamerica/manifest.json
Author
Thank you for letting me know! I ran prettier and it should be good now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description:
Comprehensive accuracy and quality-of-life improvements across 8 maps. No gameplay mechanics changed: only nation names, spawn positions, and flag codes. All changes bring maps closer to real-world geographic and historical accuracy.
Changes by map:
All changes made and verified in the map editor. No en.json or UI changes required as these are map data files only.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
@islandspiritozempic