Repository navigation
City map: window-like header with a line badge, rounder cards - #544
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpWbDt2Rc4m9DDypitMH3J
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
lmonteirotech
left a comment
There was a problem hiding this comment.
Verdict: APPROVED at b23d92f, on condition that CI is 7/7 green on this exact head (when I reviewed it, 6 of 7 checks were still running and provenance had passed). This is posted as a comment because the PR is opened from the same account as the review.
Scope: visual only, confirmed. The diff is 2 files, CHANGELOG.md and nav/city.rs. It changes only the header fill, the title and line badge, and corner radii (outlined 4→6, legend and map_card 4→8). The fields, hit rects, buttons and handlers are untouched, and the badge is drawn from the same f.line and f.terminus values as before.
Strings: there are no new keys. The title reuses the existing lt.city_map key and only drops .to_uppercase(), so the 26-language catalog stays complete.
DX9/DX11: the change goes through Painter (rect, rounded, text) only, with no renderer or shader path. It is renderer-agnostic and DX9/DX11 are unaffected.
Changelog: the entry is under Unreleased/Changed, in English, and says "nothing the map shows or does changes". It names no upstream, and the commit message is clean.
Renders: mapa-cidade-antes.png and mapa-cidade-depois.png match the diff. The header is the window panel colour with "City map" in plain case. The line is an accent badge followed by the terminus. The Trip card, legend and buttons have rounder corners, and the map content is identical.
Non-blocking nits (fine to leave or follow up):
WINDOW_BAR(0x202124) duplicatesomsi_ui::ingame::PANEL, which has the same value. Using the shared constant would stop the two drifting apart.- The badge width is not clamped to
right. A very long line label would push the terminus text out of the header, but real line numbers are short. - The "no duty" fallback text goes from 17 to 15 px, which is a small unannounced difference.
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b23d92f401
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let bw = self.fonts.width(num, 13.0 * s, Weight::Bold) + 16.0 * s; | ||
| let badge = Rect::new(tx, head.center().y - 11.0 * s, bw, 22.0 * s); | ||
| ui.rounded(badge, 5.0 * s, omsi_ui::theme::accent()); |
There was a problem hiding this comment.
Constrain the line badge to the available header width
When the line name is long or the available header space is reduced by the window size or interface scaling, the badge can extend past right and paint over the header buttons, which are drawn immediately before it. Previously, the combined line/terminus text was ellipsized to (right - tx).max(0.0) through text_in; the new badge instead uses the full line width without a bound. Cap the badge width to the remaining space and omit it when there is insufficient room so that its background and text cannot obscure the controls.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4400f91: the badge width is now capped to the room left of the header buttons, and the badge and terminus are left out when there is hardly any room (no "no duty" text shown in that case).
Generated by Claude Code
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpWbDt2Rc4m9DDypitMH3J
lmonteirotech
left a comment
There was a problem hiding this comment.
Verdict: APPROVED at 4400f91, on condition that CI is 7/7 green on this exact head (no check runs had been reported for this head when I reviewed it). This is posted as a comment because the PR is opened from the same account as the review.
The delta from b23d92f is one commit, 4400f91, in nav/city.rs only, +8/−1. It addresses my earlier nit that the badge was not clamped to the space the buttons leave:
- The badge arm now needs
right - tx >= 24*s, and the badge width ismin(room), so it can no longer run under the buttons. - When there is no room, an empty
(Some, Some)arm leaves the badge and terminus out. The old behaviour in that case was to draw them anyway, under the buttons. - The terminus text rect already uses
(right - tx).max(0), so it follows the clamped badge.
The change is layout only, with no new strings and no DX9/DX11 impact. The commit message is clean. My earlier verdict on scope, strings, the changelog and the renders still stands. The renders come from a wide window, so they don't show the narrow-window fix.
Generated by Claude Code
Requested by Leo · project thread
What changes for the player?
The city map's (Shift+M) header takes the in-game windows' panel colour; its title is "City map" in plain case (was small letter-spaced capitals), the line is a badge in the accent colour and the terminus follows it. The header buttons, the Trip and You-are-here cards and the legend have rounder corners. Nothing the map shows or does changes.
Before/after of the real map (rendered by the existing GPU test
the_city_map_draws_over_the_pictureon a software adapter, no map content needed):/mnt/project-files/mockups/polimento-ui-jogo/render-real/mapa-cidade-antes.pngandmapa-cidade-depois.png.Not in this PR: the minimap's frame, the lines panel's card, keycaps in the legend (the hint is one translated sentence), and the tilted city map.
How to test
Shift+M in a drive with a line on: look at the header, the Trip card and the legend. Open the lines panel and the terrain button: unchanged behaviour.
What does this change, and why?
nav/city.rsonly: header colour (WINDOW_BAR), title and line badge, corner radii ofoutlined, the legend andmap_card. No new texts (the title is the existing "City map" key, no longer uppercased).Checklist
cargo test -p omsi-app --test app nav::passes (and the GPU city-map test, run with--ignored)Generated by Claude Code