Skip to content

Draw large settlement icons at three quarters of their old size - #42

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

Drefvelin merged 1 commit into
mainfrom
large-settlement-icon

Conversation

@Drefvelin

@Drefvelin Drefvelin commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Large settlement and capital icons render at 120 map pixels, 75% of the previous 160.
  • Small settlements, forts, ports, airports, and battle markers stay the same size. 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 visibly smaller than before and still larger than a small settlement

Summary by CodeRabbit

  • Style
    • Reduced the large settlement icons on the map from 160 to 120 pixels.

The 160px tier crowded the map next to the smaller settlements.

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: b22ff361-c970-4194-8d58-24ced083b86d

📥 Commits

Reviewing files that changed from the base of the PR and between 89b8ebc and 6c603be.

📒 Files selected for processing (3)
  • frontend/app/lib/mapMarkers.test.ts
  • frontend/app/lib/mapMarkers.ts
  • frontend/app/lib/mapPaint.ts

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


📝 Walkthrough

Walkthrough

The large settlement marker size changes from 160 to 120 pixels. A test checks the icon and layout sizes, and the map scale comment now refers to the original 160-pixel icon.

Changes

Large Settlement Marker Size

Layer / File(s) Summary
Update large settlement marker size
frontend/app/lib/mapMarkers.ts, frontend/app/lib/mapMarkers.test.ts, frontend/app/lib/mapPaint.ts
MARKER_LARGE_PX changes to 120. The test checks that the icon and layout sizes are both 120 pixels. The map scale comment compares the stamp size with the original 160-pixel icon.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~7 minutes

Change: Bug fix

Suggested reviewers: justinasla

Merge Risk: ⚪ Minimal · up to 6c603

Large settlement markers render at 120px instead of 160px, while small markers remain 100px. The shared size is used across the inspected map views, with no unresolved merge risk.

🚥 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 75% of its previous size.
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 3…
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 hops past markers bright,
One-fifty-nine? No, one-twenty in sight.
The test checks each measured size,
The map scale note recalls the old icon’s guise.
Soft paws trace the pixels through the night.

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

@Drefvelin
Drefvelin merged commit 6f43a3f into main Sep 27, 2026
2 checks passed
@Drefvelin
Drefvelin deleted the large-settlement-icon branch September 27, 2026 17:12
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