Skip to content

Conversation

@xezon
Copy link

@xezon xezon commented Jan 31, 2026

This change always keeps the Radar View Box in sync with the actual Camera View.

Originally this worked fine for Camera Pitch and Zoom, but it would not work with FOV changes and similar unaccounted camera modifications that can be added in future changes.

The change implicitly also simplifies the code.

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Fix Is fixing something, but is not user facing labels Jan 31, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 31, 2026

Greptile Overview

Greptile Summary

Refactored radar view box synchronization to use event-based notification instead of polling. Previously, the radar checked zoom and angle on every frame to detect camera changes, but missed FOV changes and other camera modifications. The new approach notifies the radar whenever setCameraTransform() is called, ensuring the view box stays synchronized with all camera changes.

Key Changes:

  • Added notifyViewChanged() virtual method to base Radar class
  • W3DView::setCameraTransform() now calls TheRadar->notifyViewChanged() after updating camera transform
  • Removed m_viewAngle and m_viewZoom member variables from W3DRadar (no longer needed)
  • Removed per-frame polling logic that compared zoom/angle values in W3DRadar::draw()
  • Simplified code by replacing manual change detection with direct notification

Benefits:

  • Captures all camera view changes (pitch, zoom, FOV, and future modifications)
  • Cleaner architecture with event-driven design instead of polling
  • Reduced code complexity by removing redundant state tracking

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The refactoring is well-designed and improves the existing system without introducing bugs. The null check for TheRadar prevents crashes during initialization. The change replaces fragile polling with robust event notification, fixing the original FOV bug while simplifying the codebase.
  • No files require special attention

Important Files Changed

Filename Overview
Core/GameEngine/Include/Common/Radar.h Added virtual notifyViewChanged() method to base Radar class interface
Core/GameEngineDevice/Include/W3DDevice/Common/W3DRadar.h Overrode notifyViewChanged() method, removed m_viewAngle and m_viewZoom member variables, improved comment formatting
Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp Implemented notifyViewChanged(), removed manual zoom/angle comparisons from draw(), removed angle/zoom storage from reconstructViewBox() and constructor
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp Added notifyViewChanged() call in setCameraTransform() to notify radar of all camera changes, added include for Radar.h

Sequence Diagram

sequenceDiagram
    participant User
    participant W3DView
    participant Camera
    participant Radar
    participant W3DRadar

    Note over User,W3DRadar: Camera View Change Flow (NEW)
    
    User->>W3DView: Camera change (zoom/pitch/FOV/etc)
    W3DView->>W3DView: setCameraTransform()
    W3DView->>Camera: Set_Transform(cameraTransform)
    W3DView->>Camera: Set_Clip_Planes(nearZ, farZ)
    W3DView->>Camera: Set_View_Plane(m_FOV, -1)
    W3DView->>Radar: notifyViewChanged()
    Radar->>W3DRadar: notifyViewChanged()
    W3DRadar->>W3DRadar: m_reconstructViewBox = TRUE
    
    Note over User,W3DRadar: Radar Drawing (every frame)
    
    User->>W3DRadar: draw()
    alt m_reconstructViewBox == TRUE
        W3DRadar->>W3DRadar: reconstructViewBox()
        Note over W3DRadar: Computes 4 corner points<br/>of camera view in radar space
        W3DRadar->>W3DRadar: m_reconstructViewBox = FALSE
    end
    W3DRadar->>W3DRadar: drawViewBox()
    Note over W3DRadar: Draws yellow box showing<br/>camera viewport on radar
Loading

Copy link

@Mauller Mauller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sense it makes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Fix Is fixing something, but is not user facing Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants