Skip to content

Draw large settlement icons at 90 map pixels - #44

Merged
Drefvelin merged 1 commit into
mainfrom
large-settlement-90
Sep 27, 2026
Merged

Drefvelin merged 1 commit into
mainfrom
large-settlement-90

Conversation

@Drefvelin

@Drefvelin Drefvelin commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Large settlement and capital icons render at 90 map pixels, down from 120.
  • Small settlements stay at 100. Label type size is unchanged.

Test plan

  • vitest run app/lib/mapMarkers.test.ts app/lib/settlementMarkers.test.ts app/lib/map/chronicleGifFrame.test.ts
  • After production deploy, a large settlement icon is 90px on the map

Summary by CodeRabbit

  • Visual Updates
    • Large settlement markers are now smaller on the map, measuring 90 px compared with 100 px for small settlement markers. Their labels remain larger than those on small settlement markers.

The 120px tier was still larger than the settlements next to it.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9097f844-3e9f-44a6-817e-7dfcecd8b59f

📥 Commits

Reviewing files that changed from the base of the PR and between a1489bc and 36c901f.

📒 Files selected for processing (4)
  • frontend/app/lib/map/chronicleGifFrame.test.ts
  • frontend/app/lib/mapMarkers.test.ts
  • frontend/app/lib/mapMarkers.ts
  • frontend/app/lib/settlementMarkers.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The configured large settlement marker size changes from 120 to 90. Related tests now check the updated size, its ratio to a small marker, and font-size comparisons.

Changes

Settlement marker size

Layer / File(s) Summary
Update marker size and assertions
frontend/app/lib/mapMarkers.ts, frontend/app/lib/mapMarkers.test.ts, frontend/app/lib/settlementMarkers.test.ts, frontend/app/lib/map/chronicleGifFrame.test.ts
MARKER_LARGE_PX changes to 90. Tests expect a large marker size of 90, a small marker size of 100, and a large icon that is 90% the size of a small icon. The chronicle test retains its font-size comparison.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: justinasla

Merge Risk: ⚪ Minimal · up to 36c90

Large settlement icons change to 90 map pixels while small markers remain 100 pixels, matching the stated visual intent. No actionable merge risk is evident in the supplied changes.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reducing large settlement icon size to 90 map pixels.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the marker’s size
Ninety pixels meet its eyes
Small icons keep their hundred glow
Font sizes still compare below
Then hops away through fields of green

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

@Drefvelin
Drefvelin merged commit 2ff02bf into main Sep 27, 2026
2 checks passed
@Drefvelin
Drefvelin deleted the large-settlement-90 branch September 27, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants