Adds Map of Vietnam#4592
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds the Vietnam map with region metadata, team spawn areas, dimensions, thumbnail, English localization, and registration in the generated game map list. ChangesVietnam map
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/core/game/Maps.gen.ts (1)
106-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse UpperCamelCase for the
vietnamenum key and MapInfoid.All other
GameMapTypeenum keys use UpperCamelCase (e.g.,Achiran,AmazonRiver,Venice). Thevietnamkey should beVietnam, and the MapInfoidshould be"Vietnam"per the interface comment: "GameMapType enum key — the UpperCamelCase folder name." This is downstream of thename: "vietnam"issue ininfo.json— fixing the name there to "Vietnam" and regenerating would propagate the correction.Based on learnings from PR 4408, per-map test updates are not required for generated map registry additions.
♻️ Proposed fix
- vietnam = "vietnam", // map-generator/assets/maps/vietnam/info.json + Vietnam = "Vietnam", // map-generator/assets/maps/vietnam/info.json{ - id: "vietnam", - type: GameMapType.vietnam, + id: "Vietnam", + type: GameMapType.Vietnam, translationKey: "map.vietnam", categories: ["new", "countries", "asia"], multiplayerFrequency: 4, specialTeamCount: 2, },Also applies to: 897-904
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/game/Maps.gen.ts` at line 106, Update the generated map metadata so the GameMapType enum entry and corresponding MapInfo id use UpperCamelCase: rename the vietnam key to Vietnam and set its id to "Vietnam". Also update the source map info name to "Vietnam" and regenerate the affected map registry entries, without adding per-map test changes.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@map-generator/assets/maps/vietnam/info.json`:
- Line 96: Update the province name value in the Vietnam map metadata from “An
GIang” to “An Giang,” preserving all other fields unchanged.
- Around line 104-105: Update the map name value in the Vietnam map metadata
from lowercase to “Vietnam”, preserving the existing id. Regenerate the
dependent GameMapType entry in Maps.gen.ts and the English localization display
value so both use the capitalized name consistently.
In `@resources/lang/en.json`:
- Line 1020: Update the "vietnam" map localization entry in the language
resource so its display value is capitalized as "Vietnam", while preserving the
lowercase key matching the map ID.
---
Nitpick comments:
In `@src/core/game/Maps.gen.ts`:
- Line 106: Update the generated map metadata so the GameMapType enum entry and
corresponding MapInfo id use UpperCamelCase: rename the vietnam key to Vietnam
and set its id to "Vietnam". Also update the source map info name to "Vietnam"
and regenerate the affected map registry entries, without adding per-map test
changes.
🪄 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: 6fc3c160-182e-4c3b-aadb-525fb03b06b2
⛔ Files ignored due to path filters (4)
map-generator/assets/maps/vietnam/image.pngis excluded by!**/*.pngresources/maps/vietnam/map.binis excluded by!**/*.binresources/maps/vietnam/map16x.binis excluded by!**/*.binresources/maps/vietnam/map4x.binis excluded by!**/*.bin
📒 Files selected for processing (5)
map-generator/assets/maps/vietnam/info.jsonresources/lang/en.jsonresources/maps/vietnam/manifest.jsonresources/maps/vietnam/thumbnail.webpsrc/core/game/Maps.gen.ts
| { | ||
| "coordinates": [898, 3175], | ||
| "flag": "South Vietnam", | ||
| "name": "An GIang" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix "An GIang" capitalization typo.
The province name should be "An Giang" (lowercase 'g'). "GIang" is not a correct spelling of any Vietnamese province.
💚 Proposed fix
- "name": "An GIang"
+ "name": "An Giang"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "name": "An GIang" | |
| "name": "An Giang" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@map-generator/assets/maps/vietnam/info.json` at line 96, Update the province
name value in the Vietnam map metadata from “An GIang” to “An Giang,” preserving
all other fields unchanged.
| "id": "vietnam", | ||
| "name": "vietnam", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Capitalize the map name to "Vietnam".
All other maps use properly capitalized name values (e.g., "Africa", "Amazon River", "Korea"). The lowercase "vietnam" is inconsistent and cascades into the GameMapType enum key in Maps.gen.ts, which should be UpperCamelCase (Vietnam), and the localization display value in en.json.
💚 Proposed fix
- "name": "vietnam",
+ "name": "Vietnam",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "id": "vietnam", | |
| "name": "vietnam", | |
| "id": "vietnam", | |
| "name": "Vietnam", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@map-generator/assets/maps/vietnam/info.json` around lines 104 - 105, Update
the map name value in the Vietnam map metadata from lowercase to “Vietnam”,
preserving the existing id. Regenerate the dependent GameMapType entry in
Maps.gen.ts and the English localization display value so both use the
capitalized name consistently.
| "twolakes": "Two Lakes", | ||
| "unitedstates": "United States", | ||
| "venice": "Venice", | ||
| "vietnam": "vietnam", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Capitalize the map display name to "Vietnam".
All other map entries use properly capitalized display names (e.g., "Africa", "Korea", "Venice"). The value "vietnam" will appear lowercase in the map picker UI. The key "vietnam" is correct (it matches the map id), but the value should be "Vietnam".
💚 Proposed fix
- "vietnam": "vietnam",
+ "vietnam": "Vietnam",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "vietnam": "vietnam", | |
| "vietnam": "Vietnam", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/lang/en.json` at line 1020, Update the "vietnam" map localization
entry in the language resource so its display value is capitalized as "Vietnam",
while preserving the lowercase key matching the map ID.
Add approved & assigned issue number here:
Resolves #4591 (issue number)
Description:
"height": 3560,
"num_land_tiles": 1504115,
"width": 1976
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME crunchybbbbb