refactor(W3DMPO): Remove W3DMPO class to avoid unnecessary virtual classes#2607
refactor(W3DMPO): Remove W3DMPO class to avoid unnecessary virtual classes#2607Caball009 wants to merge 5 commits intoTheSuperHackers:mainfrom
Conversation
|
| 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
(link is deliberately broken for the time being)
(link is deliberately broken for the time being)
This PR removes the
W3DMPOclass because it has no value in and of itself. Originally,W3DMPOcould not be derived from by other classes without adding theW3DMPO_GLUE(due to the pure virtual function inW3DMPO) , but there was no mechanism in place to prevent the opposite (seeTextureClass).Check commits for cleaner diff.
TODO: