Conversation
…rkers Arma directions are clockwise from north, but the rotation matrix used counter-clockwise convention. Negate the angle to match Arma's convention. Added test coverage for rotation direction.
Only show markers belonging to the currently selected side (+ GLOBAL). Lifted activeSide signal to shared shortcuts module so MarkerManager can react to side changes via the render bridge.
Vehicles now display crew info matching the old frontend format: "VehicleName (N)\nCrew1\nCrew2...". Added HTML escaping and tests.
Matches old frontend behavior: popups are hidden at zoom <=4 (legacy) or <=14 (MapLibre). Also refreshes popup visibility on nameDisplayMode change.
Builds popup labels for ICON markers matching old frontend logic: player-owned markers show "SIDE PlayerName Text", system markers show just the text, projectiles show "PlayerName Text".
…le/briefing) ICON markers are now classified into systemMarkers (player=-1), projectileMarkers (magIcons/Minefield/mil_triangle on GLOBAL), or briefingMarkers (player-owned) matching the old frontend logic.
Vehicle popups now use text-align: left (leaflet-popup-vehicle) matching the old frontend, while infantry popups keep text-align: center (leaflet-popup-unit).
Matches the old frontend's getCrewString() which filtered crew names to isPlayer=true units only. AI crew members are counted in the total but not listed by name.
Vehicles now derive isPlayer from crew composition: true if any crew member is a player, false otherwise. This matches the old frontend's hideMarkerPopup logic which checked vehicle crew for player presence.
Summary of ChangesHello @fank, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refines the user interface and display logic within the recording playback feature. It addresses several visual inconsistencies and enhances the clarity of map elements. Key improvements include accurate rotation for briefing markers, dynamic filtering of markers based on selected sides, and more informative vehicle popups that detail player crew. Additionally, it optimizes map performance and readability by managing popup visibility according to zoom levels and correctly categorizing markers into distinct rendering layers. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Coverage Report for ui
File Coverage |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of fixes and enhancements to the recording playback UI. The changes, including corrected marker rotations, improved vehicle popups with crew details, and refined marker filtering, significantly enhance the user experience. The code is well-structured and accompanied by thorough tests for each new feature and fix. I've identified a potential bug related to popup visibility logic and have also suggested a few refactorings to improve code clarity and maintainability. Overall, this is a high-quality contribution.
- Store isPlayer/isInVehicle on InternalMarkerHandle so refreshPopupVisibility applies the full visibility logic on zoom and nameDisplayMode changes, preventing brief popup flicker. - Use Array.some() for vehicleHasPlayerCrew (more idiomatic). - Group GLOBAL conditions in buildMarkerPopupText and use PROJECTILE_TYPES constant consistently.
openPopup() on a marker not yet added to the map has no effect because the popup DOM element doesn't exist yet. Move openPopup() after addTo().
Vehicle positions in legacy JSON use run-length encoding with [startFrame, endFrame] ranges at index 4. The decoder was treating each RLE entry as a single frame, causing vehicles to appear at wrong positions (e.g. a vehicle stationary for 13 minutes would jump to its 13-minute position at ~44 seconds of playback). Now correctly expands RLE entries into one EntityState per frame, matching the Go backend's collectEntityPositions behavior.
removeBriefingMarker() was hardcoded to remove from briefingMarkers layer group, but markers classified as projectileMarkers (smoke grenades, ammo icons) or systemMarkers were added to different groups. Leaflet's removeLayer() silently no-ops when the layer isn't in the group, so these markers never disappeared. Use L.Layer.remove() instead, which removes from whatever parent the layer is currently in — matching the old frontend's removeMarker() behavior.
Summary
leaflet-popup-vehicleCSS class for vehicle popups (left-aligned text, min-width 200px)isPlayerderives from crew composition for "players" display mode visibilityrefreshPopupVisibilityTest plan