Skip to content

refactor(W3DMPO): Remove W3DMPO class to avoid unnecessary virtual classes#2607

Draft
Caball009 wants to merge 5 commits intoTheSuperHackers:mainfrom
Caball009:refactor_W3DMPO_GLUE
Draft

refactor(W3DMPO): Remove W3DMPO class to avoid unnecessary virtual classes#2607
Caball009 wants to merge 5 commits intoTheSuperHackers:mainfrom
Caball009:refactor_W3DMPO_GLUE

Conversation

@Caball009
Copy link
Copy Markdown

@Caball009 Caball009 commented Apr 15, 2026

This PR removes the W3DMPO class because it has no value in and of itself. Originally, W3DMPO could not be derived from by other classes without adding the W3DMPO_GLUE (due to the pure virtual function in W3DMPO) , but there was no mechanism in place to prevent the opposite (see TextureClass).

Check commits for cleaner diff.

TODO:

  • Replicate in Generals.

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels Apr 15, 2026
@Caball009 Caball009 marked this pull request as ready for review April 15, 2026 22:49
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 15, 2026

Greptile Summary

This PR removes the W3DMPO base class and renames W3DMPO_GLUE to W3DMPO_CODE across GeneralsMD/ and Core/ files, stripping the unnecessary virtual dispatch mechanism (glueEnforcer()) from the memory-pool macro. The refactoring is mechanically consistent across all 46 modified files, but 21 files in Generals/Code/Libraries/Source/WWVegas/WW3D2/ still inherit from the now-deleted W3DMPO class and still invoke the now-undefined W3DMPO_GLUE macro — breaking the vanilla Generals build.

Confidence Score: 3/5

Not safe to merge as-is — the Generals (vanilla) build is broken by the removal of W3DMPO_GLUE and the W3DMPO class without migrating the 21 affected Generals files.

A P1 build-breaking change exists: 21 files in Generals/ still reference the deleted W3DMPO class and W3DMPO_GLUE macro. The GeneralsMD changes are clean and correct, but the shared always.h modification breaks the Generals build at this commit.

Core/Libraries/Source/WWVegas/WWLib/always.h — removal of W3DMPO_GLUE/W3DMPO class breaks the Generals build; the 21 unmigrated files under Generals/Code/Libraries/Source/WWVegas/WW3D2/ need attention.

Important Files Changed

Filename Overview
Core/Libraries/Source/WWVegas/WWLib/always.h Core change: removes W3DMPO class, renames W3DMPO_GLUE to W3DMPO_CODE; contains a "first first" typo in a comment and unusually omits backslash-continuations on multi-line comment lines inside the macro
Core/Libraries/Source/WWVegas/WW3D2/dllist.h DLNodeClass no longer inherits W3DMPO; destructor loses its virtual qualifier — safe because DLListClass always deletes through T* and CRTP ensures concrete types are known
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/mapper.h TextureMapperClass drops W3DMPO base; concrete subclasses each carry W3DMPO_CODE — correct since the base is never directly instantiated
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/render2d.h Render2DClass drops W3DMPO inheritance, retains its own virtual destructor; W3DMPO_CODE added correctly
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/hlod.h HLodClass, HLodDefClass, HLodPrototypeClass cleanly migrated; virtual destructor override annotations removed appropriately
GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/motchan.h All five motion-channel classes correctly migrated to W3DMPO_CODE
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Core/Libraries/Source/WWVegas/WWLib/always.h
Line: 133-148

Comment:
**Vanilla Generals build broken — `W3DMPO_GLUE` and `W3DMPO` removed with no compatibility shim**

`W3DMPO_GLUE` is no longer defined anywhere, and the `W3DMPO` class has been deleted. Twenty-one files in `Generals/Code/Libraries/Source/WWVegas/WW3D2/` (e.g. `mapper.h`, `hlod.h`, `motchan.h`, `mesh.h`, `boxrobj.h` …) still inherit from `W3DMPO` and invoke `W3DMPO_GLUE(…)`. These files will fail to compile as soon as they include the updated `always.h` via `corei_always`.

The PR description acknowledges this with a TODO, but renaming the macro and deleting the class in the same commit means the Generals build is broken at merge time. The minimal fix is to keep a backward-compatible alias:

```cpp
// Deprecated alias – remove once Generals is migrated
#define W3DMPO_GLUE(ARGCLASS) W3DMPO_CODE(ARGCLASS)
class W3DMPO { };
```

or to migrate the Generals files in this same PR.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Core/Libraries/Source/WWVegas/WWLib/always.h
Line: 137-140

Comment:
**Typo: "first first" and missing `\` on comment lines inside macro**

Line 138 has a doubled word ("first first"). The intended phrase is likely "construct on first use". Additionally, the comment lines inside this `#define` lack the backslash-continuation that was present on every line in the original `W3DMPO_GLUE`. While technically correct (the block comment spans the missing `\` newlines during phase-3 tokenization), the omission is surprising and departs from the explicit style of the original.

```suggestion
		/*
		Construct on first use to avoid static initialization order fiasco
		that may occur if this were initialized prior to the initialization of TheMemoryPoolFactory.
		*/ \
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Removed 'virtual' & 'override' from dest..." | Re-trigger Greptile

Comment thread Core/Libraries/Source/WWVegas/WWLib/always.h
@Caball009 Caball009 marked this pull request as draft April 16, 2026 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Minor Severity: Minor < Major < Critical < Blocker 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.

1 participant