Repository navigation
Conversation
Confetti3
left a comment
There was a problem hiding this comment.
Thanks for this. Having the chat input on the room board next to the messages is a good change: the draft, the acknowledgement path and the history screen are all shared with the existing code, and the controller handling (Back keeps the draft, shortcuts wait while you type) is thought through. Two defects need fixing before it merges, plus a missing translation and two structural suggestions.
1. The composer can be pushed out of view in the wide layout. In src/ui/RoomPanel.cxx:663 the community pane gets max(120*s, boardHeight - toolsHeight - gap), and the member list above the chat always takes at least one whole row (:673). DrawRoomChat (src/ui/ChatPanel.cxx:95) then draws its heading and a full input row whatever is left, and only the transcript shrinks (:106, clamped to 1 pixel). With a normal private room at 1920x1080 and 200% interface scaling the toolbar takes four rows, so the pane sits at its minimum height and the input and Send button end up below the pane's clip rectangle. Selecting inline-chat with a controller does not scroll the pane, so it cannot be reached either. Please reserve the composer's height (heading plus input row) before sizing the member list, and fall back to the stacked arrangement when the community pane cannot hold the members heading, one member row and the composer. The journey test checks the clip rectangle but not this size; please add 1920x1080 at 200% with assertions that both the input and Send lie inside the pane.
2. Changing rooms while the box is active keeps typing focus. On a room change ObserveChat clears roomChatTyping_ and roomChatFocus_ (src/ui/ChatPanel.cxx:61), and RoomEntries clears the draft, but ImGui's active ID for ##room-chat-draft is untouched. If the board stays on screen across the replacement, the same widget is still active, IsItemActive() sets roomChatTyping_ again (src/ui/ChatPanel.cxx:140), and the next characters go into the new room's draft without the player choosing its box. Please release the real editor ownership on the epoch change (clear the active ID when it is this widget, and apply the neutral-input gate there), or include the room epoch in the widget ID. The test at src/tests/shell_chat_journey.hxx:157 changes rooms after Back has already released focus, so it does not cover this; please add a case that changes the epoch while the box is active and checks that focus is released.
3. The new strings are untranslated. chat.history and chat.inline_detail are added in English to all 13 other locale files (for example locales/de.po:3595). chat.history replaces the label of the existing Chat row, so players on those locales now see an English row where there was a translation. Please translate both, or at least keep the previous translated wording for chat.history.
4. Suggestion: share the composer with the history screen. DrawRoomChat repeats what DrawChatScreen does (src/ui/ChatPanel.cxx:283 onward): the buffer copy, ReloadUserBufAndMoveToEnd, CaretToEnd, keeping the draft on Escape and syncing chat_. Ownership of Enter and Back is now split between roomChatHeld_, the shell's per-frame NeutralGate() call (src/ui/ApplicationShell.cxx:810) and ImGui's active ID, which is also how item 2 slipped through. One small composer used by both screens, returning explicit "sent" and "released" results, would keep the two from drifting. The two screens' different initial-focus behaviour can stay a parameter.
5. Suggestion: resolve the board's space once. compactBoard and shortBoard (src/ui/RoomPanel.cxx:421) now reach into pane placement, the strip label size, the explanation row and the card height. In particular line 714 shortens table cards to the list height, but tableCard still lays out its header, footer and banner in fixed bands (band=h-60*s-bannerHeight), so a card shorter than about 60*s plus the banner gets a zero or negative portrait size. It would be simpler to measure the minimum size of each pane, reserve the composer first (see item 1), and let the scrolling list take any shortage, keeping cards at their normal height.
Still needs a gameplay check in a real room: typing and sending from keyboard and controller, a room replacement while the box is focused, and the narrow and 200% layouts.
Extract this focused change from the locally tested development build. Keep other features out of the upstream review; the pull request records implementation details, prior integration evidence, and outstanding isolated validation.
56ec874 to
fef9216
Compare
Keep every key and translated value intact while avoiding unrelated PR additions at the shared country-list tail.
Use persistent window focus and one transcript visibility check for full and compact chat. Keep offscreen arrivals unread while scrolling, behind dialogs or while unfocused.
Room chat can be read and sent directly from the main room board. Chat history remains available for longer conversations; both views use the existing shared draft, acknowledged-send and recovery/retry path.
Review fixes
F/T/C, Delete and menu navigation stay out of the way while the inline editor owns input. Enter/Send uses the same authoritative send handler as history, and an acknowledged message clears the shared draft.
Restoration checks (17 October 2026)
Restored the same final source revision that was present before withdrawal. The target
releasebranch is unchanged atcd4daa5c2fb19ee5a3ecd14d0e2d9cbee753aa4f. All six restored submissions (#44, #51, #98, #100, #102 and #104) passgit diff --check, merge individually with that release, and merge cleanly in all 15 pairwise combinations.Review #44 before #100, and #51 plus #104 before #102. The dependent branches contain those exact prerequisite heads; after a squash/rebase merge, refresh the dependents as described below. #98 is independent of those sequences.
This restoration did not change code or rerun builds/gameplay. The build, rendering and network-test results below are previously recorded evidence, including earlier combined tests with other contribution branches that remain closed.
Validation
Remaining acceptance
A real room still needs keyboard/controller sending, focus replacement, narrow/high-scale visual inspection and multiplayer delivery on this isolated branch. Automated shell/render checks do not establish that native multiplayer acceptance. Discord-SDK-enabled build remains unrun on this machine.
Chat read tracking and current compatibility
The shared read-tracking fix from #104 is included here. The newest message must actually be visible, the game must have focus, and no dialog may cover the conversation. Compact chat additionally requires mouse attention or controller focus on its composer. The old inline
MarkReadshortcut is removed so it cannot bypass those checks. Expiring a notification never acknowledges unread chat.This compatibility pass built the combined x86 Sidecar (Discord SDK and helper rebuilding disabled) and passed all 11 selected CTests without failures or skips: ShellJourney, RoomPanelNavigation, ControllerNavigation, Localization, OverlayPresentation, OverlayLifecycle, MenuUsability, SettingsNavigation, UiRender, UiRenderLocales and UiRenderAtlas. New inline regressions cover offscreen arrivals and lost focus in all four existing layouts. Restoring the old inline read shortcut makes the focus regression fail; restoring production code passes.
Across the then-current 17 contribution PR heads, 136/136 pairs merge cleanly and three complete merge orders produce the same source tree. These checks preserve commit history. Simulated squash merges of the shared prerequisites create conflicts; refresh remaining dependent branches against upstream after a squash/rebase merge. New upstream edits require another compatibility check.
The native injected-game and physical-controller spectator/chat acceptance remains outstanding. This pass did not publish a new playable package or change chat delivery, room protocols or GGPO.
#104 provides the four-second spectator/Training notification and compact badge separately. The inline-chat feature and that notification PR can be reviewed independently; their shared read-tracking commit is identical.