Skip to content

refactor: Add override keyword to virtual function overrides in GameEngineDevice#2393

Open
bobtista wants to merge 6 commits intoTheSuperHackers:mainfrom
bobtista:bobtista/override-keyword-gameenginedevice
Open

refactor: Add override keyword to virtual function overrides in GameEngineDevice#2393
bobtista wants to merge 6 commits intoTheSuperHackers:mainfrom
bobtista:bobtista/override-keyword-gameenginedevice

Conversation

@bobtista
Copy link

@bobtista bobtista commented Mar 3, 2026

Summary

  • Add override keyword to virtual function overrides in GameEngineDevice
  • Covers W3DDevice, Win32Device, StdDevice, VideoDevice
  • Changes across Core, Generals, and GeneralsMD

Context

Part 5/6 of splitting #2101. Depends on #2389 merging first.

Notes

  • 113 files changed, purely mechanical override keyword additions
  • Includes bugfix: remove incorrect override from methods without matching virtual base class
  • All lines retain the virtual keyword

@greptile-apps
Copy link

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR is part 5/6 of a mechanical refactoring series (split from #2101) that adds the override keyword to virtual function overrides across 113 files in the GameEngineDevice layer covering W3DDevice, Win32Device, StdDevice, and VideoDevice subsystems across Core, Generals, and GeneralsMD.

The changes are a net positive for code health: the override specifier causes compile-time errors if a method signature drifts out of sync with its base class, making future refactoring safer. A secondary benefit is that several destructors which were previously non-virtual (e.g. ~BinkVideoPlayer(), ~W3DDisplay(), ~W3DView()) have been correctly promoted to virtual ... override, closing potential slicing/double-delete hazards.

Key observations:

  • All added override keywords have been verified against their respective base classes (confirmed for MultiListObjectClass, HashableClass, VideoPlayerInterface, AudioManager, TerrainVisual, etc.).
  • A small set of methods in W3DTerrainVisual.h (addFactionBib, removeFactionBib, addFactionBibDrawable, removeFactionBibDrawable, getLogicHeightMap, getClientHeightMap) also have virtual added alongside override, which is correct since they were overriding base-class virtuals without being explicitly declared virtual.
  • Methods correctly omitted from override in W3DView.h (isDoingScriptedCamera, stopDoingScriptedCamera, cameraEnableRealZoomMode, cameraDisableRealZoomMode, cameraModFreezeTime, isTimeFrozen) are confirmed to be W3DView-exclusive virtual introductions, not overrides.
  • A handful of pre-existing virtual methods in MilesAudioManager.h (audioDebugDisplay, addAudioEvent, processPlayingList, etc.) do override base-class virtuals but were intentionally left unchanged, consistent with the incremental series approach.
  • Shadow-file destructors (~W3DShadowTexture, ~W3DShadowGeometry) gain override without virtual — this is valid C++11 (the override implicitly retains virtuality) but is slightly inconsistent in style with the rest of the PR.

Confidence Score: 5/5

  • This PR is safe to merge — all changes are purely additive override annotations with no behavioral impact.
  • The changes are purely mechanical keyword additions. The override specifier is enforced at compile time, so any incorrect application (missing base-class virtual) would have caused a build failure before this PR could land. No logic, data, or ABI is altered. The few destructors promoted to virtual were already called polymorphically through virtual dispatch (their base classes had virtual destructors), so behaviour is unchanged.
  • No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngineDevice/Include/MilesAudioDevice/MilesAudioManager.h Adds override to ~40 virtual method declarations. A handful of pre-existing virtual methods (audioDebugDisplay, addAudioEvent, findPlayingAudioFrom, processPlayingList, etc.) were not touched and still lack override, though these appear to be intentionally out-of-scope for this PR.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DTerrainVisual.h Adds override to all virtual methods. Also correctly adds the virtual specifier to previously non-virtual methods (addFactionBib, removeFactionBib, addFactionBibDrawable, removeFactionBibDrawable, getLogicHeightMap, getClientHeightMap) which override base-class virtuals — the override keyword would have produced a compile error if these were wrong.
Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DShaderManager.cpp Adds override to all anonymous shader classes (ShroudTextureShader, FlatShroudTextureShader, MaskTextureShader, TerrainShader2Stage, etc.) that inherit from W3DShaderInterface and W3DFilterInterface. Changes are purely mechanical and correct.
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DProjectedShadow.cpp Adds override to W3DShadowTexture and MissingTextureClass destructors and Get_Key() methods. Correctly validates against HashableClass which has virtual ~HashableClass() and virtual const char* Get_Key() = 0.
Core/GameEngineDevice/Include/VideoDevice/Bink/BinkVideoPlayer.h Promotes ~BinkVideoPlayer() from non-virtual to virtual ... override, which is correct since VideoPlayerInterface (grandparent via VideoPlayer) declares virtual ~VideoPlayerInterface() {}. All other override additions are accurate.
Core/GameEngineDevice/Include/W3DDevice/GameClient/Module/W3DModelDraw.h Adds override to ~40 virtual methods. setAnimationFrame(int frame) is correctly left without override as it appears to be a W3DModelDraw-specific non-overriding method.
Generals/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DDisplay.h Mechanical override additions across ~30 virtual method declarations. Also correctly upgrades the destructor from ~W3DDisplay() (non-virtual) to virtual ~W3DDisplay() override.
Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DView.h Largest single-file change (~65 methods). Methods correctly left without override (isDoingScriptedCamera, stopDoingScriptedCamera, cameraModFreezeTime, isTimeFrozen, cameraEnableRealZoomMode, cameraDisableRealZoomMode) are W3DView-specific virtual methods not present in the View base class.
Core/GameEngineDevice/Include/W3DDevice/GameClient/CameraShakeSystem.h Single change: ~CameraShakerClass()virtual ~CameraShakerClass() override. Correct — MultiListObjectClass declares virtual ~MultiListObjectClass().
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/Shadow/W3DVolumetricShadow.cpp Mirrors the Generals version: adds override to W3DShadowGeometry and MissingGeomClass destructors and Get_Key() methods. Changes are correct.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class AudioManager {
        +virtual void init()
        +virtual void audioDebugDisplay()
        +virtual AudioHandle addAudioEvent()
    }
    class MilesAudioManager {
        +virtual void init() override
        +virtual void openDevice() override
        +virtual void closeDevice() override
        +virtual void stopAudio() override
    }
    AudioManager <|-- MilesAudioManager

    class VideoPlayerInterface {
        +virtual void init()
        +virtual void deinit()
        +virtual ~VideoPlayerInterface()
    }
    class VideoPlayer {
        +virtual void init()
        +virtual ~VideoStream()
    }
    class BinkVideoPlayer {
        +virtual void init() override
        +virtual void deinit() override
        +virtual ~BinkVideoPlayer() override
    }
    VideoPlayerInterface <|-- VideoPlayer
    VideoPlayer <|-- BinkVideoPlayer

    class TerrainVisual {
        +virtual void addFactionBib()
        +virtual WorldHeightMap* getLogicHeightMap()
    }
    class W3DTerrainVisual {
        +virtual void addFactionBib() override
        +virtual WorldHeightMap* getLogicHeightMap() override
    }
    TerrainVisual <|-- W3DTerrainVisual

    class HashableClass {
        +virtual ~HashableClass()
        +virtual const char* Get_Key()
    }
    class W3DShadowTexture {
        +~W3DShadowTexture() override
        +virtual const char* Get_Key() override
    }
    HashableClass <|-- W3DShadowTexture

    class View {
        +virtual void init()
        +virtual void drawView()
    }
    class W3DView {
        +virtual void init() override
        +virtual void drawView() override
        +virtual bool isDoingScriptedCamera()
    }
    View <|-- W3DView
Loading

Last reviewed commit: 47bfd7e

@bobtista bobtista force-pushed the bobtista/override-keyword-gameenginedevice branch from 1c9931f to 37bcbc7 Compare March 9, 2026 20:09
@xezon xezon added the Refactor Edits the code with insignificant behavior changes, is never user facing label Mar 10, 2026
Copy link

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

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

Looks good

@xezon xezon changed the title refactor(GameEngineDevice): Add override keyword to virtual function overrides refactor: Add override keyword to virtual function overrides in GameEngineDevice Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Refactor Edits the code with insignificant behavior changes, is never user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants