Skip to content

Conversation

@Duwibi
Copy link
Contributor

@Duwibi Duwibi commented Dec 27, 2025

Description:

Fixes 2 of the unconnected river tiles on the Lisbon map.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

DISCORD_USERNAME
Duwibi

@Duwibi Duwibi requested a review from a team as a code owner December 27, 2025 13:22
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Duwibi Duwibi added the Maps A new map, or adjustments to an existing map itself, its json, etc, label Dec 27, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 27, 2025

Walkthrough

The pull request updates the land tile counts in the Lisbon map manifest file. Three map variants (map, map16x, map4x) have their num_land_tiles values adjusted downward to reflect corrected tile counts. No structural changes or logic modifications were made.

Changes

Cohort / File(s) Summary
Lisbon Map Manifest
resources/maps/lisbon/manifest.json
Updated num_land_tiles values for map (1495857), map16x (90194), and map4x (369383) variants

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

  • #2422: Updates num_land_tiles across map manifest variants using identical change pattern
  • #1445: Corrects num_land_tiles values in map manifest files to reflect accurate tile counts
  • #2545: Directly modifies num_land_tiles in resources/maps/lisbon/manifest.json

Suggested labels

Feature - Map

Suggested reviewers

  • evanpelle
  • jrouillard

Poem

🗺️ In Lisbon's tile-lands far and wide,
Numbers shift with careful pride,
Three maps refined, precision gleams,
Corrected counts fulfill our dreams! 🎯

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix the Lisbon map' is directly related to the changeset, which updates the Lisbon map manifest with corrected tile counts for fixing unconnected river tiles.
Description check ✅ Passed The description is related to the changeset, explaining that it fixes unconnected river tiles on the Lisbon map, which corresponds to the manifest updates in the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9d9a32 and f31c066.

⛔ Files ignored due to path filters (2)
  • map-generator/assets/maps/lisbon/image.png is excluded by !**/*.png
  • resources/maps/lisbon/map.bin is excluded by !**/*.bin
📒 Files selected for processing (1)
  • resources/maps/lisbon/manifest.json
🔇 Additional comments (1)
resources/maps/lisbon/manifest.json (1)

4-4: Verify the tile count proportionality across map variants.

The land tile counts show scaling deviations beyond typical rounding:

  • map4x is 1.2% below expected (369383 vs ~369964 expected)
  • map16x is 3.5% below expected (90194 vs ~93491 expected)

Other maps in the repo (plains, half_land_half_ocean, ocean_and_land) maintain near-perfect 4:1 and 16:1 ratios. If these variant maps are generated by consistent downscaling, run a tile-count validation tool to confirm the numbers are accurate.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Duwibi Duwibi added this to the v28 milestone Dec 27, 2025
@evanpelle evanpelle merged commit cd5751d into openfrontio:main Dec 27, 2025
10 of 18 checks passed
evanpelle pushed a commit that referenced this pull request Jan 2, 2026
## Description:
Fixes 2 of the unconnected river tiles on the Lisbon map.

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [x] I have added relevant tests to the test directory
- [x] I confirm I have thoroughly tested these changes and take full
responsibility for any bugs introduced

## Please put your Discord username so you can be contacted if a bug or
regression is found:

DISCORD_USERNAME
Duwibi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maps A new map, or adjustments to an existing map itself, its json, etc,

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants