refactor(w3dview): Migrate from MEMBER_ADD/MEMBER_RELEASE macros to RefCountPtr<T> #1763

bobtista · 2025-10-31T00:51:54Z

Merge after refactor(w3dview): Replace duplicate reference count macros with core library equivalents #1784

Replaces custom reference counting macros (MEMBER_ADD, MEMBER_RELEASE, SAFE_ADD_REF, SAFE_RELEASE_REF) with the type-safe RefCountPtr smart pointer from the core library.

Member Variables Migrated to RefCountPtr:

AssetInfoClass::m_pRenderObj
ScreenCursorClass::m_pTexture, m_pVertMaterial
PlaySoundDialogClass::SoundObj
SpherePropertySheetClass::m_RenderObj
RingPropertySheetClass::m_RenderObj
CW3DViewDoc::m_pCRenderObj, m_pCAnimation, m_pCAnimCombo, m_pCursor, m_pCursorScene
CGraphicView::m_pCamera, m_pLightMesh

Local Variable Usage:

Replaced MEMBER_ADD/MEMBER_RELEASE with REF_PTR_SET/REF_PTR_RELEASE for local raw pointers in 15 files

Removed Macros (Utils.h):

MEMBER_ADD
MEMBER_RELEASE
SAFE_ADD_REF
SAFE_RELEASE_REF

Notes:

Used Create_AddRef() when taking ownership of an existing pointer
Used Create_NoAddRef() when wrapping factory methods that return ref=1
Added .Peek() calls when passing to functions expecting raw pointers
Updated getter methods to return .Peek() instead of raw RefCountPtr
Preserved all original logic and formatting

xezon

Before I continue review, please make another pass on this and correct all missing resource cleanups. It is important that no logic is changed, unless it fixes a bug.

Core/Tools/W3DView/W3DViewDoc.cpp

xezon · 2025-11-02T16:38:23Z

Core/Tools/W3DView/W3DViewDoc.cpp


    // Was there a valid display object?
    if (m_pCRenderObj)
    {


The condition above is obsoleted.

So, yes the operations inside are safe to execute regardless. If I understand correctly, the conditional checks make the intent explicit (only cleanup when there's something to clean up). RefCountPtr provides operator const DummyPtrType *() specifically to enable null-checking in boolean contexts.
// From ref_ptr.h documentation (lines 92-96):
void Do_It(void)
{
if (MyObject) { // Recommended pattern
MyObject->Do_Something();
}
}

This is also the pattern we use elsewhere eg CW3DViewDoc::CleanupResources.

Happy to keep or remove them, up to you

Assignment functions already do the tests and are safe.

GeneralsGameCode/Core/Libraries/Source/WWVegas/WWLib/ref_ptr.h

Lines 289 to 298 in 7a6402b

const RefCountPtr<T> & operator =(DummyPtrType * dummy_ptr)

{

if (Referent) {

Referent->Release_Ref();

}

Referent = 0;

return *this;

}

Alternatively, use Clear() function. Maybe that is cleaner.

GeneralsGameCode/Core/Libraries/Source/WWVegas/WWLib/ref_ptr.h

Lines 352 to 358 in 7a6402b

void Clear(void)

{

if (Referent) {

Referent->Release_Ref();

Referent = 0;

}

}

I like Clear :) Done

Core/Tools/W3DView/W3DViewDoc.h

xezon

How about we first make a change where we just remove the duplicate Macros, without adding RefCountPtr? Then this change will be simpler? And then make small changes to introduce RefCountPtr slowly? I am afraid this will be a big review here otherwise given the implications. One mistake with ref counting and the game will crash.

Core/Tools/W3DView/W3DViewDoc.cpp

bobtista · 2025-11-03T00:40:18Z

How about we first make a change where we just remove the duplicate Macros, without adding RefCountPtr? Then this change will be simpler? And then make small changes to introduce RefCountPtr slowly? I am afraid this will be a big review here otherwise given the implications. One mistake with ref counting and the game will crash.

Ok, check out 1784. Shall we continue this PR after rebasing and have it focus just on just W3DView, then additional RefCountPtr migrations can have separate PRs?

xezon · 2025-11-22T17:38:26Z

This still needs work.

xezon

This refactor needs another pass. Something went wrong the indentation of code. And behavior has changed.

Core/Tools/W3DView/AssetInfo.cpp

Core/Tools/W3DView/W3DViewDoc.cpp

Core/Tools/W3DView/EmitterInstanceList.cpp

Core/Tools/W3DView/PlaySoundDialog.cpp

Core/Tools/W3DView/RingPropertySheet.cpp

xezon · 2025-11-22T20:00:08Z

Core/Tools/W3DView/W3DViewDoc.cpp

-    if (m_pCBackObjectScene)
-    {
-        // Free the scene object
-        m_pCBackObjectScene->Release_Ref ();


m_pCBackObjectScene.Clear() missing?

m_pCBackObjectScene is a raw pointer (SceneClass *), not a RefCountPtr, so it uses Release_Ref() and = NULL for cleanup (lines 167-168). Want me to convert it to RefCountPtr? Or should that be in a separate PR?

Checking again - On main:
m_pCursorScene used REF_PTR_RELEASE and was migrated to RefCountPtr with .Clear().
m_pCBackObjectScene used manual Release_Ref() + = NULL (not REF_PTR_RELEASE), so it wasn’t done in this PR. Do you want me to migrate it in this PR?

Core/Tools/W3DView/W3DViewDoc.cpp

bobtista · 2025-12-03T16:24:12Z

Core/Tools/W3DView/W3DViewDoc.cpp

 		SAFE_DELETE (m_pCAnimCombo);
-		REF_PTR_RELEASE (m_pCAnimation);
-
-			if (m_pCRenderObj != NULL) {


This indentation seems wrong in the original code, fixed here

bobtista · 2025-12-03T16:34:06Z

I think this is ready for another look

bobtista · 2025-12-15T20:39:40Z

Core/Tools/W3DView/W3DViewDoc.cpp

        DisplayObject (pCModel);

        // Get an instance of the animation object
-		  SAFE_DELETE (m_pCAnimCombo);


Here too, the indentation was weird in the original. It looks like they meant to do:
Function body level: 4 spaces
Inside the if block: 8 spaces (4 + 4)
But a few spots got 10 spaces, eg SAFE_DELETE, so I fixed those, at least here

…o RefCountPtr<T> Replace MEMBER_ADD/MEMBER_RELEASE macros with RefCountPtr<T> smart pointers. - AssetInfo: m_pRenderObj now RefCountPtr<RenderObjClass> - ScreenCursor: m_pTexture and m_pVertMaterial now RefCountPtr - PlaySoundDialog: SoundObj now RefCountPtr<AudibleSoundClass>

Replace MEMBER_ADD/MEMBER_RELEASE macros with RefCountPtr<T> smart pointers. - SpherePropertySheet: m_RenderObj now RefCountPtr<SphereRenderObjClass> - RingPropertySheet: m_RenderObj now RefCountPtr<RingRenderObjClass> - EmitterPropertySheet: local emitter variables use REF_PTR_RELEASE - EmitterInstanceList: use REF_PTR_RELEASE for vector cleanup

Replace MEMBER_ADD/MEMBER_RELEASE macros with RefCountPtr<T> and REF_PTR_RELEASE. - W3DViewDoc: m_pCRenderObj, m_pCAnimation, m_pCursorScene, m_pCursor now RefCountPtr - GraphicView: m_pCamera and m_pLightMesh now RefCountPtr - Local variables in Utils, DataTreeView, BoneMgrDialog, etc. use REF_PTR_RELEASE

Remove obsolete macros now replaced by RefCountPtr<T> and core library macros. - Removed: MEMBER_ADD, MEMBER_RELEASE, SAFE_ADD_REF, SAFE_RELEASE_REF - Kept: SAFE_DELETE, SAFE_DELETE_ARRAY, COM_RELEASE, SAFE_CLOSE, SANITY_CHECK

…rScene in CleanupResources

… checks

…s to RefCountPtr idiomatic form

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch 8 times, most recently from cc1d8dc to c3f8d36 Compare October 31, 2025 02:22

Skyaero42 added Refactor Edits the code with insignificant behavior changes, is never user facing Stability Concerns stability of the runtime labels Nov 1, 2025

xezon reviewed Nov 2, 2025

View reviewed changes

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch 2 times, most recently from 0cbd2fc to 4c2acf0 Compare November 2, 2025 18:28

xezon reviewed Nov 2, 2025

View reviewed changes

bobtista mentioned this pull request Nov 3, 2025

refactor(w3dview): Replace duplicate reference count macros with core library equivalents #1784

Merged

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch from 3c0d862 to d3cd83c Compare November 6, 2025 17:18

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch from 1229310 to 52e834f Compare November 22, 2025 18:17

xezon reviewed Nov 22, 2025

View reviewed changes

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch 4 times, most recently from 81a71ad to bad267f Compare December 3, 2025 16:23

bobtista commented Dec 3, 2025

View reviewed changes

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch from bad267f to e2d2ae2 Compare December 15, 2025 20:17

bobtista commented Dec 15, 2025

View reviewed changes

bobtista added 2 commits January 14, 2026 16:20

bobtista added 17 commits January 14, 2026 16:21

fix(w3dview): add missing NULL assignments for m_pCursor and m_pCurso…

5ed03af

…rScene in CleanupResources

nit: revert indentation change

4d691e7

refactor(w3dview): use RefCountPtr::Clear() and remove redundant null…

ab0c388

… checks

fix(w3dview): move RefCountPtr::Clear() calls into proper scope

4937dd9

fix(w3dview): use .Peek() in ASSERT macros for explicit pointer checking

07ee845

remove duplicate cleanup of m_pCBackObjectScene

a2e5ecd

fix(w3dview): add missing cleanup for local render object

c59fcf8

fix(w3dview): add missing RefCountPtr cleanups and fix indentation

2bbc396

remove extra include, fix indentation

c91b881

formatting change

8d46655

remove extra clear call

8a7b329

formatting if block

4dba9bc

style(w3dview): split single-line if statements to multi-line format

412b990

nit: indentation fixes

9a7e54c

fix: indentation using tabs instead of spaces, updated two NULL check…

9bdab84

…s to RefCountPtr idiomatic form

bobtista force-pushed the bobtista/refactor-w3dview-refcountptr branch from e2d2ae2 to 9bdab84 Compare January 14, 2026 21:32

	const RefCountPtr<T> & operator =(DummyPtrType * dummy_ptr)
	{
	if (Referent) {
	Referent->Release_Ref();
	}

	Referent = 0;

	return *this;
	}

	void Clear(void)
	{
	if (Referent) {
	Referent->Release_Ref();
	Referent = 0;
	}
	}

refactor(w3dview): Migrate from MEMBER_ADD/MEMBER_RELEASE macros to RefCountPtr<T> #1763

Are you sure you want to change the base?

refactor(w3dview): Migrate from MEMBER_ADD/MEMBER_RELEASE macros to RefCountPtr<T> #1763

Uh oh!

Conversation

bobtista commented Oct 31, 2025 • edited by xezon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobtista commented Nov 3, 2025

Uh oh!

xezon commented Nov 22, 2025

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobtista commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bobtista commented Oct 31, 2025 •

edited by xezon

Loading

xezon Nov 22, 2025 •

edited

Loading