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

[Caball009]

• Closes issue Optimize class W3DMPO to remove virtual functions in release mode #1948
• Merge after PR refactor: Add override keyword to virtual function overrides in Core code (2) #2603

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.

[greptile-apps]

Greptile Summary

This PR removes the W3DMPO base class and its associated pure-virtual glueEnforcer enforcement mechanism, replacing all W3DMPO_GLUE(X) call sites with a new W3DMPO_CODE(X) macro that retains the custom memory-pool operator new/delete without introducing a virtual dispatch layer. The change spans 67 files across Core, Generals, and GeneralsMD.

• always.h: W3DMPO class deleted; W3DMPO_GLUE renamed to W3DMPO_CODE with the virtual void glueEnforcer() enforcement dropped and the block comment style updated.
• All class sites: Inheritance from W3DMPO removed and virtual ~X() override downgraded to plain ~X() where W3DMPO was the sole provider of the virtual destructor; classes with other polymorphic bases retain virtual destruction through those bases.
• DLNodeClass<T>: Template base class loses its virtual destructor; safe under current CRTP-style usage but leaves a subtle trap for future code deleting nodes through a DLNodeClass<T>* pointer.

Confidence Score: 5/5

Safe to merge; the refactoring is mechanically consistent across all 67 files and no code paths are functionally altered.

The change is a purely structural refactor: the custom memory-pool plumbing is preserved exactly, only the virtual-dispatch enforcement layer is removed. Every call site has been updated consistently. The non-standard comment style inside the macro and DLNodeClass losing its virtual destructor do not introduce regressions in current code.

Core/Libraries/Source/WWVegas/WWLib/always.h (macro comment style and typo) and Core/Libraries/Source/WWVegas/WW3D2/dllist.h (non-virtual destructor on base class template).

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WWLib/always.h	Core change: renames W3DMPO_GLUE to W3DMPO_CODE, removes the W3DMPO base class and its pure-virtual glueEnforcer; new comment omits backslash continuation on the /* line and has a double-word typo.
Core/Libraries/Source/WWVegas/WW3D2/dllist.h	DLNodeClass no longer inherits W3DMPO; loses its virtual destructor — safe for current DLListClass usage but a subtle trap for future subclasses deleted through DLNodeClass* pointers.
Generals/Code/Libraries/Source/WWVegas/WW3D2/mapper.h	Removes W3DMPO inheritance from TextureMapperClass and all concrete mapper subclasses; W3DMPO_GLUE replaced with W3DMPO_CODE on each concrete class.
Generals/Code/Libraries/Source/WWVegas/WW3D2/motchan.h	Five motion-channel classes drop W3DMPO inheritance and virtual destructors; safe as none form their own polymorphic hierarchy.
Generals/Code/Libraries/Source/WWVegas/WW3D2/hlod.h	HLodClass, HLodDefClass, and HLodPrototypeClass drop W3DMPO; HLodPrototypeClass retains virtual destruction via PrototypeClass.
Generals/Code/Libraries/Source/WWVegas/WW3D2/meshmdlio.cpp	MeshLoadContextClass drops W3DMPO and virtual destructor; private to the translation unit so no external code deletes through a base pointer.
Generals/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp	SortingNodeStruct gains W3DMPO_CODE and drops W3DMPO; CRTP pattern ensures deletions occur through the concrete type.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class W3DMPO {
        <<removed>>
        +virtual ~W3DMPO()
        #virtual glueEnforcer() const = 0
    }
    class W3DMPO_CODE {
        <<macro>>
        -getClassMemoryPool() void*
        +operator new(size_t) void*
        +operator delete(void*) void
    }
    class HLodClass {
        W3DMPO_CODE(HLodClass)
    }
    class TextureMapperClass
    class ScaleTextureMapperClass {
        W3DMPO_CODE(ScaleTextureMapperClass)
    }
    class MotionChannelClass {
        W3DMPO_CODE(MotionChannelClass)
    }
    class DLNodeClass~T~ {
        ~DLNodeClass()
    }
    class SortingNodeStruct {
        W3DMPO_CODE(SortingNodeStruct)
    }
    W3DMPO_CODE ..> HLodClass : injected
    W3DMPO_CODE ..> ScaleTextureMapperClass : injected
    W3DMPO_CODE ..> MotionChannelClass : injected
    W3DMPO_CODE ..> SortingNodeStruct : injected
    TextureMapperClass <|-- ScaleTextureMapperClass
    DLNodeClass~T~ <|-- SortingNodeStruct

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
Core/Libraries/Source/WWVegas/WWLib/always.h:137-140
Double word typo in the new comment — "first first use" should be "first use".

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

### Issue 2 of 3
Core/Libraries/Source/WWVegas/WWLib/always.h:137-140
**Block comment continuation without backslash in macro body**

The old code used `/* \` — explicit `\` on every line — to keep the block comment syntactically inside the `#define` replacement list. The new `/*` on line 137 has no trailing `\`, so according to the standard the logical `#define` directive ends there, leaving the rest of the macro body (lines 141–148) technically at file scope rather than inside the macro.

In practice GCC, Clang, and MSVC all treat a `/*` inside a macro as consuming subsequent physical lines until `*/`, so this compiles today. That behaviour is a common extension rather than conforming C++, so the change trades portability for a slightly cleaner comment style. Consider restoring `/* \` to stay strictly conforming.

### Issue 3 of 3
Core/Libraries/Source/WWVegas/WW3D2/dllist.h:90
**Virtual destructor removed from intended base class**

`DLNodeClass<T>` is explicitly designed to be subclassed (all its list traversal helpers return `T*` via `static_cast`). Previously, inheriting from `W3DMPO` gave it a virtual destructor. It now has a non-virtual `~DLNodeClass()`. Any code that stores these nodes as `DLNodeClass<T>*` and calls `delete` through that pointer will not invoke the derived-class destructor, silently leaking any resources owned by the concrete node type (e.g. `SortingNodeStruct`).

`DLListClass` itself never calls `delete` on nodes, so no current path is obviously broken, but the unsafe contract is worth a comment and a virtual destructor to make the class safe for future subclasses.

_{Reviews (3): Last reviewed commit: "Replicated in Generals." | Re-trigger Greptile}

[Caball009]

Generals is broken until the changes are replicated there.

[xezon]

Instead of removing the class, how about removing the macro, and making W3DMPO a template class instead? It looks as if that should work, even in VC6.

[Caball009]

Instead of removing the class, how about removing the macro, and making W3DMPO a template class instead? It looks as if that should work, even in VC6.

Could you expand on that?

[xezon]

I checked my suggestion and it does not satisfy the requirements.

The approach in this change is the correct one.

While reviewing, I realized that many classes inherit RefCountClass for reference counting, which has a virtual void Delete_This() { delete this; } function and is not overloaded by W3DMPO classes. I wonder how this works then. Will delete this call the overloaded delete functions? I assume it does, otherwise the runtime should fail catastophically, but I do not know how it knows which overloaded delete to call. They are not virtual functions after all.

Can you check it for a potential follow up fix?

[Caball009]

RefCountClass::Delete_This calls the destructor, which is virtual, so it knows how to call the destructor of the class that inherited from RefCountClass.

[xezon]

RefCountClass::Delete_This calls the destructor, which is virtual, so it knows how to call the destructor of the class that inherited from RefCountClass.

Yes, but we want to call this:

inline void operator delete(void *p) { freeFromW3DMemPool(getClassMemoryPool(), p); }

If it does not call this, then how does it free it from the correct mem pool?

[Caball009]

This is what happens if I'm not mistaken.

1. RefCountClass::Delete_This -> calls delete this on base class.
2. RefCountClass has a virtual destructor, so it uses the vtable to find the 'last' (most derived) destructor.
3. The compiler has generated a tiny wrapper function around the destructor, so it knows which operator delete to call eventually, and stored (a pointer to) the wrapper function in the vtable.
   The wrapper function is basically the encapsulation of delete, which calls the destructor first and then operator delete.
4. So instead of calling the destructor directly, this wrapper function is called.
5. The wrapper calls actual the destructor first (which works its way 'upward' to the destructor of the base class).
6. The wrapper then calls operator delete.