Skip to content

Some minor changes #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: x86-64-branch
Choose a base branch
from
Open

Some minor changes #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
075b72b
tier1: allow one to implement custom convar register functions
RaphaelIT7 May 14, 2025
032ee4b
tier0: remove function for windows since tier0 already contains them
RaphaelIT7 May 9, 2025
bf0c565
tier1: change it to actually be tier1
RaphaelIT7 May 18, 2025
01fa228
tier1: fix this inline
RaphaelIT7 Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions public/tier0/threadtools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,7 @@ class PLATFORM_CLASS CThreadRWLock
class ALIGN8 PLATFORM_CLASS CThreadSpinRWLock
{
public:
#ifndef WIN32
Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

What are these changes trying to do?

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025

Choose a reason for hiding this comment

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

iirc I had some linker issues on Windows where these were already defined in the tier0.lib or another lib file & caused a pain for a while until the best decision seemed to just wrap it with WIN32 which worked.

CThreadSpinRWLock()
{
m_lockInfo.m_i32 = 0;
Expand All @@ -1689,6 +1690,9 @@ class ALIGN8 PLATFORM_CLASS CThreadSpinRWLock
m_iWriteDepth = 0;
#endif
}
#else
CThreadSpinRWLock();
#endif

bool IsLockedForWrite();
bool IsLockedForRead();
Expand Down Expand Up @@ -2452,13 +2456,15 @@ inline bool CThreadSpinRWLock::TryLockForWrite_UnforcedInline()
#endif
}

#ifndef WIN32
FORCEINLINE void CThreadSpinRWLock::LockForWrite()
{
if ( !TryLockForWrite() )
{
SpinLockForWrite();
}
}
#endif

FORCEINLINE bool CThreadSpinRWLock::TryLockForRead()
{
Expand Down Expand Up @@ -2497,6 +2503,7 @@ inline bool CThreadSpinRWLock::TryLockForRead_UnforcedInline()
return TryLockForRead();
}

#ifndef WIN32
FORCEINLINE void CThreadSpinRWLock::LockForRead()
{
if ( !TryLockForRead() )
Expand Down Expand Up @@ -2545,6 +2552,7 @@ void CThreadSpinRWLock::UnlockRead()
}
#endif
}
#endif

#else
/* (commented out to reduce distraction in colorized editor, remove entirely when new implementation settles)
Expand Down
12 changes: 12 additions & 0 deletions public/tier1/convar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ class ConCommandBase

// ConVars in this executable use this 'global' to access values.
static IConCommandBaseAccessor *s_pAccessor;

public:
inline ConCommandBase* InternalNext() { return m_pNext; };
static inline ConCommandBase* InternalConCommandBases() { return s_pConCommandBases; };
static inline IConCommandBaseAccessor* InternalBaseAccessor() { return s_pAccessor; };
};


Expand Down Expand Up @@ -1055,6 +1060,13 @@ class SplitScreenSlottedConVarRef : public SplitScreenConVarRef
void ConVar_Register( int nCVarFlag = 0, IConCommandBaseAccessor *pAccessor = NULL );
void ConVar_Unregister( );

//-----------------------------------------------------------------------------
// Utility methods when implementing your custom ConVar_Register function.
//-----------------------------------------------------------------------------
int* ConVar_GetConVarFlag();
int* ConVar_GetDLLIdentifier();
bool* ConVar_GetIsRegistered();
IConCommandBaseAccessor* ConVar_GetDefaultAccessor();

//-----------------------------------------------------------------------------
// Utility methods
Expand Down
28 changes: 26 additions & 2 deletions tier1/convar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
#endif

// This enables the l4d style of culling all cvars that are not marked FCVAR_RELEASE :
#define CULL_ALL_CVARS_NOT_FCVAR_RELEASE
// RaphaelIT7: Fk this, why would anyone want this in a SDK.
// #define CULL_ALL_CVARS_NOT_FCVAR_RELEASE
Comment on lines -35 to +36
Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

What's the purpose of this? Can't you just add a FCVAR_RELEASE flag to the convar/concommand?

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025

Choose a reason for hiding this comment

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

iirc FCVAR_RELEASE doesn't exist on 32x so if modules don't consider this, they have random issues with their convars being invisible/never registered.

Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

I can add a simple

#ifndef FCVAR_RELEASE
#define FCVAR_RELEASE 0
#endif

like in the haptics files for the main branch.

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It would work, though still on 32x by default this isn't a thing, and adding that would still cause useless debugging/pain when its missing and it doesn't really seem to be necessary to have this for modules.

Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

I like to avoid making incompatible changes or with different behavior from the original. I'm way beyond the "hopeful" part for the x86-64 branch becoming the main one, but still... Defining a new FCVAR_RELEASE on the main branch is the solution that deviates the least from the regular behavior. If people develop modules for the x86-64 branch, then they must be aware of the caveats of using it.

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 28, 2025

Choose a reason for hiding this comment

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

Maybe having a flag or something to disable this on x86-64 would be useful instead of having to add this on 32x everywhere.

Copy link
Owner

@danielga danielga Jul 28, 2025

Choose a reason for hiding this comment

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

You don't have to add the flag on modules that only use the main branch. If you use the x86-64 branch, then you add the flag, simple. It will do nothing when you build the module for the main branch.


//-----------------------------------------------------------------------------
// Statically constructed list of ConCommandBases,
Expand All @@ -44,6 +45,21 @@ static int s_nCVarFlag = 0;
static int s_nDLLIdentifier = -1; // A unique identifier indicating which DLL this convar came from
static bool s_bRegistered = false;

int* ConVar_GetConVarFlag()
{
return &s_nCVarFlag;
}

int* ConVar_GetDLLIdentifier()
{
return &s_nDLLIdentifier;
}

bool* ConVar_GetIsRegistered()
{
return &s_bRegistered;
}
Comment on lines +48 to +61
Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

These don't seem specially useful, as they won't return the real instances from Garry's Mod.

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025

Choose a reason for hiding this comment

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

Example use case: https://github.com/RaphaelIT7/gmod-holylib/blob/main/source/util.cpp#L691
Using ConVar_GetDLLIdentifier, for example, I can iterate over all convars and find all those from my DLL easily.


class CDefaultAccessor : public IConCommandBaseAccessor
{
public:
Expand All @@ -57,6 +73,12 @@ class CDefaultAccessor : public IConCommandBaseAccessor

static CDefaultAccessor s_DefaultAccessor;

IConCommandBaseAccessor* ConVar_GetDefaultAccessor()
{
return &s_DefaultAccessor;
}

#ifndef TIER1_CUSTOMCONVARREGISTER
Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

Do you have an example of these custom functions?

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025

Choose a reason for hiding this comment

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

I had used them a good while ago, though they then lost their purpose and were removed again.
Tbh I think I can remove the TIER1_CUSTOMCONVARREGISTER stuff since its really not that useful as I first had thought it would be

//-----------------------------------------------------------------------------
// Called by the framework to register ConCommandBases with the ICVar
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -87,7 +109,9 @@ void ConVar_Register( int nCVarFlag, IConCommandBaseAccessor *pAccessor )

ConCommandBase::s_pConCommandBases = NULL;
}
#endif

#ifndef TIER1_CUSTOMCONVARUNREGISTER
void ConVar_Unregister( )
{
if ( !g_pCVar || !s_bRegistered )
Expand All @@ -101,7 +125,7 @@ void ConVar_Unregister( )
s_nDLLIdentifier = -1;
s_bRegistered = false;
}

#endif

//-----------------------------------------------------------------------------
// Purpose: Default constructor
Expand Down
4 changes: 4 additions & 0 deletions tier1/premake5_create_project.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ group("SourceSDK")
["Header files/*"] = {"../public/tier1/*.h", "../public/*.h", "../common/xbox/*.h"}
})

if TIER1_PROJECTCALLBACK then
TIER1_PROJECTCALLBACK()
end
Comment on lines +130 to +132
Copy link
Owner

@danielga danielga Jul 27, 2025

Choose a reason for hiding this comment

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

I don't like this workaround, specially for something that doesn't seem to be a problem.

Copy link
Contributor Author

@RaphaelIT7 RaphaelIT7 Jul 27, 2025

Choose a reason for hiding this comment

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

Same as above, I might remove this again.


IncludeSDKCommonInternal()
IncludeSDKLZMA()

Expand Down