Skip to content

refactor: Add override keyword to virtual function overrides in Core code (2)#2603

Open
Caball009 wants to merge 10 commits intoTheSuperHackers:mainfrom
Caball009:override_keyword_core
Open

refactor: Add override keyword to virtual function overrides in Core code (2)#2603
Caball009 wants to merge 10 commits intoTheSuperHackers:mainfrom
Caball009:override_keyword_core

Conversation

@Caball009
Copy link
Copy Markdown

@Caball009 Caball009 commented Apr 15, 2026

This PR adds the keyword override to a number of virtual functions in the Core code that were missed in the previous round of refactoring.

Check out the commits as they separate different types of changes.

Related PRs:
#2604
#2605

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

This PR continues the override keyword refactoring started in #2101, adding override (and in some cases const) to virtual function declarations across ~70 Core header files. Beyond mechanical override additions, several substantive corrections are bundled in: redundant re-declarations of init/update/reset in interface sub-classes are removed (they propagate from SubsystemInterface), amIHost() gains const-correctness, some sound methods align their const-ness with the base class, and CardinalSpline1DClass::Add_Key adopts the three-parameter base signature.

  • texture.h / always.h (coupled fix): W3DMPO_GLUE's glueEnforcer() now carries override, which requires W3DMPO in the inheritance chain. TextureClass was the only W3DMPO_GLUE user missing W3DMPO as a direct base; adding it fixes what would have been a compile error once always.h changed.
  • Generals/DrawModule.h: setAnimationFrame(int frame) = 0 is added as a pure virtual, bringing the Generals variant into parity with the already-updated GeneralsMD variant and allowing W3DModelDraw::setAnimationFrame to use override correctly.
  • VelocityPanelHemisphere::performUpdate is missing the virtual keyword that all sibling panels received, a minor style inconsistency left by the refactor pass.

Confidence Score: 5/5

Safe to merge — changes are exclusively override annotations and const-correctness fixes; the only non-trivial structural change adds W3DMPO to TextureClass, which pairs correctly with the always.h macro change.

All override additions verified against base-class signatures. The coupled texture.h/always.h changes are internally consistent and fix a latent mismatch. Removed redundant pure-virtual re-declarations are still enforced through SubsystemInterface. The missing virtual keyword on VelocityPanelHemisphere::performUpdate has no runtime impact.

Core/Libraries/Source/WWVegas/WW3D2/texture.h and Core/Libraries/Source/WWVegas/WWLib/always.h should be reviewed together since their changes are tightly coupled.

Important Files Changed

Filename Overview
Core/Libraries/Source/WWVegas/WW3D2/texture.h Adds W3DMPO as a base class of TextureClass — a structural fix required by the W3DMPO_GLUE macro whose glueEnforcer() now uses override in always.h; all other W3DMPO_GLUE users already had W3DMPO in their hierarchy, making TextureClass the sole exception.
Core/Libraries/Source/WWVegas/WWLib/always.h Adds override to glueEnforcer() inside W3DMPO_GLUE macro; tightly coupled to the texture.h fix.
Core/Libraries/Source/WWVegas/WWMath/cardinalspline.h Adds override and aligns Add_Key signature with the base HermiteSpline1DClass three-parameter version; correct.
Core/GameEngine/Include/Common/ArchiveFileSystem.h Removes redundant init/update/reset re-declarations and adds override to postProcessLoad().
Core/GameEngine/Include/GameNetwork/LANGameInfo.h Promotes amIHost() to virtual const override, matching the GameInfo base; implementation switches to getConstLANSlot() correctly.
Generals/Code/GameEngine/Include/Common/DrawModule.h Adds setAnimationFrame(int frame) = 0 as pure virtual to match GeneralsMD, enabling W3DModelDraw::setAnimationFrame override.
Core/Tools/ParticleEditor/VelocityTypePanels.h Adds override to all panel methods; VelocityPanelHemisphere::performUpdate omits virtual keyword, minor inconsistency.
Core/GameEngine/Include/Common/GameMemory.h Adds override to destructor and getObjectMemoryPool() in MEMORY_POOL_GLUE macros; correct.
Core/Libraries/Source/WWVegas/WWAudio/FilteredSound.h Adds const and override to Get_Class_ID(), matching the const base-class declaration.
Core/GameEngineDevice/Include/W3DDevice/GameClient/Module/W3DTreeDraw.h Adds override to getAsW3DTreeDrawModuleData(); base declaration present in both Generals and GeneralsMD Module.h.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class SubsystemInterface {
        +virtual init() = 0
        +virtual reset() = 0
        +virtual update() = 0
        +virtual postProcessLoad()
    }
    class ArchiveFileSystem {
        +virtual postProcessLoad() override = 0
    }
    class W3DMPO {
        +virtual glueEnforcer() const = 0
    }
    class TextureBaseClass
    class TextureClass {
        +W3DMPO_GLUE(TextureClass)
    }
    class GameInfo {
        +virtual amIHost() const
    }
    class LANGameInfo {
        +virtual amIHost() const override
    }
    class DrawModuleGenerals {
        +virtual setAnimationFrame(int frame) = 0
    }
    class W3DModelDraw {
        +virtual setAnimationFrame(int frame) override
    }
    SubsystemInterface <|-- ArchiveFileSystem
    W3DMPO <|-- TextureClass
    TextureBaseClass <|-- TextureClass
    GameInfo <|-- LANGameInfo
    DrawModuleGenerals <|-- W3DModelDraw
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/Tools/ParticleEditor/VelocityTypePanels.h:94
All sibling panels in this file (and in EmissionTypePanels.h / ParticleTypePanels.h) use `virtual void performUpdate(...) override`, but `VelocityPanelHemisphere` has only `void performUpdate(...) override``virtual` was not added here. While `override` alone is sufficient for correctness (the function remains virtual), the inconsistency is likely an oversight in the refactor pass.

```suggestion
		virtual void performUpdate( IN Bool toUI ) override;
```

Reviews (9): Last reviewed commit: "Added missing override keywords (third p..." | Re-trigger Greptile

@xezon xezon changed the title refactor: Add override keyword to virtual function overrides in Core code refactor: Add override keyword to virtual function overrides in Core code (2) Apr 16, 2026
Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

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

It looks like there are a few actual fixes for bugged overrides.

virtual void update() = 0;
virtual void reset() = 0;
virtual void postProcessLoad() = 0;
virtual void postProcessLoad() override = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if this was removed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Nothing. It's just to acknowledge that the base class also has this exact function, but it's not pure virtual. If that were removed that would lead to a compilation error here, which seems desirable.

}

Bool LANGameInfo::amIHost()
Bool LANGameInfo::amIHost() const
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this change behavior? Meaning a different function is called now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Looks like a bug to me, but without noticeable issues:

Code

Bool GameInfo::amIHost() const
{
DEBUG_ASSERTCRASH(m_inGame, ("Looking for game slot while not in game"));
if (!m_inGame)
return false;
return getConstSlot(0)->isPlayer(m_localIP);
}

Bool GameSlot::isPlayer( UnsignedInt ip ) const
{
return (m_state == SLOT_PLAYER && m_IP == ip);
}

Bool LANGameInfo::amIHost()
{
DEBUG_ASSERTCRASH(m_inGame, ("Looking for game slot while not in game"));
if (!m_inGame)
return false;
return getLANSlot(0)->isLocalPlayer();
}

Bool LANGameSlot::isLocalPlayer() const
{
return isHuman() && TheLAN && TheLAN->GetLocalIP() == getIP();
}

virtual RenderObjClass * Clone() const override;
virtual int Class_ID() const override;
virtual void Render(RenderInfoClass & rinfo) = 0;
virtual void Render(RenderInfoClass & rinfo) override = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if this is omitted?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Nothing. It's just to acknowledge that the base class also has this exact function, but it's not pure virtual. If that were removed that would lead to a compilation error here, which seems desirable.

Comment thread Core/Tools/W3DView/ViewerAssetMgr.h Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks like it does not belong here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed, but the code will fail in Generals because of it until #2604 is merged.

@xezon
Copy link
Copy Markdown

xezon commented Apr 17, 2026

Needs rebase

Comment thread Core/Tools/ParticleEditor/ParticleTypePanels.h Outdated
@Caball009 Caball009 force-pushed the override_keyword_core branch from e39b56f to 3db25c4 Compare May 2, 2026 22:22
@Caball009
Copy link
Copy Markdown
Author

Needs rebase

Rebased, but not sure if it went without a hitch.

@xezon
Copy link
Copy Markdown

xezon commented May 4, 2026

Rebase again.

@Caball009 Caball009 force-pushed the override_keyword_core branch from 10d71ec to 5e6f6bb Compare May 4, 2026 20:18
Comment thread Generals/Code/GameEngine/Include/Common/Module.h
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 4, 2026

Want your agent to iterate on Greptile's feedback? Try greploops.

@Caball009
Copy link
Copy Markdown
Author

Rebase again.

Should be good now.

@Caball009
Copy link
Copy Markdown
Author

I think I missed one or two cases in Gen / ZH. Should I just put them in this PR?

@xezon
Copy link
Copy Markdown

xezon commented May 5, 2026

I think I missed one or two cases in Gen / ZH. Should I just put them in this PR?

Yes you can.

@Caball009
Copy link
Copy Markdown
Author

FYI I'm seeing this warning 'X' hides overloaded virtual function [-Woverloaded-virtual] for the following virtual functions still, because they have a different return type and / or function parameters than the function whose name they share in the base class.

ActAsDozerState::onExit
ActAsSupplyTruckState::onExit
CFancyToolbar::Create
ChinookHeadOffMapState::onExit
DynamicMeshModel::Compute_Plane_Equations
DynamicMeshModel::Compute_Vertex_Normals
MessageStream::appendMessage
MessageStream::insertMessage
ParticleEmitterClass::Save
PopupSlider::Create
Radar::draw
RingRenderObjClass::Set_Alpha
SphereRenderObjClass::Set_Alpha
StateVectorClass::Resize
W3DAssetManager::Get_Texture

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.

2 participants