alliedmodders / hl2sdk

Half-Life 2 SDK Mirrors
https://github.com/valvesoftware
341 stars 158 forks source link

Add KeyValues3 #177

Closed vanz666 closed 7 months ago

vanz666 commented 7 months ago

This class is used in IGameEvent/CEntityKeyValues ​​and may be useful in the future.

In addition, the CUtlLeanVector* classes and some other related things have been updated.

GAMMACASE commented 7 months ago

Tested out the code, and it works well, also nice that it's finally got combined into one piece (talking about events being kv3 as well as entitykeyvalues), where apparently what I was reversing for CEntityKeyValues was the CKeyValues3Table, so tested it out on it and works fine, as well as working with event data directly via kv3.

GAMMACASE commented 7 months ago

Once you address few little issues I've pointed out, I'll happily merge that in, and again, huge thanks for the astonishing work you did!

vanz666 commented 7 months ago

I think everything looks good now.

GAMMACASE commented 7 months ago

Avoiding memdbg include seems to be more of a hacky workaround rather than a fix, do you have any ideas/solutions on how to solve it without removing that include? I don't really mind it being that way, just in case you have any other ideas about it

vanz666 commented 7 months ago

Avoiding memdbg include seems to be more of a hacky workaround rather than a fix, do you have any ideas/solutions on how to solve it without removing that include? I don't really mind it being that way, just in case you have any other ideas about it

Honestly, I don't see what problems this could cause in this section of the code, since Valve uses this hack in some places in its code. I could change the constructor to a regular function but I think it would look wrong due to the fact that the kv object is not constructed before.

GAMMACASE commented 7 months ago

If there are cases of that in the valve's code as well then I guess its fine

GAMMACASE commented 7 months ago

Do you plan on any follow up final commits, or is it ready for the merge?

vanz666 commented 7 months ago

Do you plan on any follow up final commits, or is it ready for the merge?

Everything is ready now.

inolen commented 6 months ago

Hey guys,

pinging this as I hit some build errors with this change due to some mismatched integer types in CUtlLeanVectorFixedGrowableBase -

public/tier1/utlleanvector.h:317:33: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
  317 |         if ( m_nAllocationCount > N )
      |              ~~~~~~~~~~~~~~~~~~~^~~

This class had two templates parameters that were at odds - one was a type used for the internal m_Size and m_nAllocationCount members, and the other was a size_t integer that was compared against these members.

I'm not sure where exactly this code comes from (is it from an official SDK?) or what the policy is with revolving these kinds of warnings, but here's my patch to resolve the warnings.

diff --git a/public/tier1/keyvalues3.h b/public/tier1/keyvalues3.h
index 537c6ba2..8572fd06 100644
--- a/public/tier1/keyvalues3.h
+++ b/public/tier1/keyvalues3.h
@@ -524,7 +524,7 @@ public:
    void Purge( bool bClearingContext );

 private:
-   typedef CUtlLeanVectorFixedGrowable<KeyValues3*, 4, int> ElementsVec_t;
+   typedef CUtlLeanVectorFixedGrowable<KeyValues3*, int, 4> ElementsVec_t;

    int             m_nClusterElement;
    ElementsVec_t   m_Elements;  
@@ -558,10 +558,10 @@ public:
    void Purge( bool bClearingContext );

 private:
-   typedef CUtlLeanVectorFixedGrowable<unsigned int, 8, int>   HashesVec_t;
-   typedef CUtlLeanVectorFixedGrowable<KeyValues3*, 8, int>    MembersVec_t;
-   typedef CUtlLeanVectorFixedGrowable<const char*, 8, int>    NamesVec_t;
-   typedef CUtlLeanVectorFixedGrowable<bool, 8, int>           IsExternalNameVec_t;
+   typedef CUtlLeanVectorFixedGrowable<unsigned int, int, 8>   HashesVec_t;
+   typedef CUtlLeanVectorFixedGrowable<KeyValues3*, int, 8>    MembersVec_t;
+   typedef CUtlLeanVectorFixedGrowable<const char*, int, 8>    NamesVec_t;
+   typedef CUtlLeanVectorFixedGrowable<bool, int, 8>           IsExternalNameVec_t;

    int m_nClusterElement;

@@ -717,9 +717,9 @@ public:
    void Purge();

 protected:
-   typedef CUtlLeanVectorFixedGrowable<CKeyValues3Cluster*, 8, int>        KV3ClustersVec_t;
-   typedef CUtlLeanVectorFixedGrowable<CKeyValues3ArrayCluster*, 4, int>   ArrayClustersVec_t;
-   typedef CUtlLeanVectorFixedGrowable<CKeyValues3TableCluster*, 4, int>   TableClustersVec_t;
+   typedef CUtlLeanVectorFixedGrowable<CKeyValues3Cluster*, int, 8>        KV3ClustersVec_t;
+   typedef CUtlLeanVectorFixedGrowable<CKeyValues3ArrayCluster*, int, 4>   ArrayClustersVec_t;
+   typedef CUtlLeanVectorFixedGrowable<CKeyValues3TableCluster*, int, 4>   TableClustersVec_t;

    CKeyValues3Context*         m_pContext;
    CUtlBuffer                  m_BinaryData;
diff --git a/public/tier1/utlleanvector.h b/public/tier1/utlleanvector.h
index 5de859c1..ec995926 100644
--- a/public/tier1/utlleanvector.h
+++ b/public/tier1/utlleanvector.h
@@ -158,7 +158,7 @@ inline void CUtlLeanVectorBase<T, I>::Purge()
    m_nAllocationCount = 0;
 }

-template< class T, size_t N, class I >
+template< class T, class I, I N >
 class CUtlLeanVectorFixedGrowableBase
 {
 public:
@@ -193,14 +193,14 @@ protected:
 //-----------------------------------------------------------------------------
 // constructor, destructor
 //-----------------------------------------------------------------------------
-template< class T, size_t N, class I >
-inline CUtlLeanVectorFixedGrowableBase<T, N, I>::CUtlLeanVectorFixedGrowableBase() : m_Size(0),
+template< class T, class I, I N >
+inline CUtlLeanVectorFixedGrowableBase<T, I, N>::CUtlLeanVectorFixedGrowableBase() : m_Size(0),
    m_nAllocationCount(N)
 {
 }

-template< class T, size_t N, class I >
-inline CUtlLeanVectorFixedGrowableBase<T, N, I>::~CUtlLeanVectorFixedGrowableBase()
+template< class T, class I, I N >
+inline CUtlLeanVectorFixedGrowableBase<T, I, N>::~CUtlLeanVectorFixedGrowableBase()
 {
    Purge();
 }
@@ -208,8 +208,8 @@ inline CUtlLeanVectorFixedGrowableBase<T, N, I>::~CUtlLeanVectorFixedGrowableBas
 //-----------------------------------------------------------------------------
 // Gets the base address (can change when adding elements!)
 //-----------------------------------------------------------------------------
-template< class T, size_t N, class I >
-inline T* CUtlLeanVectorFixedGrowableBase<T, N, I>::Base()
+template< class T, class I, I N >
+inline T* CUtlLeanVectorFixedGrowableBase<T, I, N>::Base()
 {
    if ( m_nAllocationCount )
    {
@@ -222,8 +222,8 @@ inline T* CUtlLeanVectorFixedGrowableBase<T, N, I>::Base()
    return NULL;
 }

-template< class T, size_t N, class I >
-inline const T* CUtlLeanVectorFixedGrowableBase<T, N, I>::Base() const
+template< class T, class I, I N >
+inline const T* CUtlLeanVectorFixedGrowableBase<T, I, N>::Base() const
 {
    if ( m_nAllocationCount )
    {
@@ -239,8 +239,8 @@ inline const T* CUtlLeanVectorFixedGrowableBase<T, N, I>::Base() const
 //-----------------------------------------------------------------------------
 // Makes sure we have enough memory allocated to store a requested # of elements
 //-----------------------------------------------------------------------------
-template< class T, size_t N, class I >
-void CUtlLeanVectorFixedGrowableBase<T, N, I>::EnsureCapacity( int num, bool force )
+template< class T, class I, I N >
+void CUtlLeanVectorFixedGrowableBase<T, I, N>::EnsureCapacity( int num, bool force )
 {
    I nMinAllocationCount = ( 31 + sizeof( T ) ) / sizeof( T );
    I nMaxAllocationCount = (std::numeric_limits<I>::max)();
@@ -295,8 +295,8 @@ void CUtlLeanVectorFixedGrowableBase<T, N, I>::EnsureCapacity( int num, bool for
 //-----------------------------------------------------------------------------
 // Element removal
 //-----------------------------------------------------------------------------
-template< class T, size_t N, class I >
-void CUtlLeanVectorFixedGrowableBase<T, N, I>::RemoveAll()
+template< class T, class I, I N >
+void CUtlLeanVectorFixedGrowableBase<T, I, N>::RemoveAll()
 {
    T* pElement = Base();
    const T* pEnd = &pElement[ m_Size ];
@@ -309,8 +309,8 @@ void CUtlLeanVectorFixedGrowableBase<T, N, I>::RemoveAll()
 //-----------------------------------------------------------------------------
 // Memory deallocation
 //-----------------------------------------------------------------------------
-template< class T, size_t N, class I >
-inline void CUtlLeanVectorFixedGrowableBase<T, N, I>::Purge()
+template< class T, class I, I N >
+inline void CUtlLeanVectorFixedGrowableBase<T, I, N>::Purge()
 {
    RemoveAll();

@@ -725,7 +725,7 @@ void CUtlLeanVectorImpl<B, T, I>::DestructElements( T* pElement, const T* pEnd )
 template < class T, class I = short >
 using CUtlLeanVector = CUtlLeanVectorImpl< CUtlLeanVectorBase< T, I >, T, I >;

-template < class T, size_t N = 3, class I = short >
-using CUtlLeanVectorFixedGrowable = CUtlLeanVectorImpl< CUtlLeanVectorFixedGrowableBase< T, N, I >, T, I >;
+template < class T, class I = short, I N = 3 >
+using CUtlLeanVectorFixedGrowable = CUtlLeanVectorImpl< CUtlLeanVectorFixedGrowableBase< T, I, N >, T, I >;

 #endif // UTLLEANVECTOR_H
GAMMACASE commented 6 months ago

@inolen we do have -Wno-sign-compare used in the build scripts, so I'm not sure if that needs fixing via the code or if we care enough for it? @psychonic would like to hear your opinion on that.

inolen commented 6 months ago

Hmm, how does that get set? It was my first night fiddling with getting a metamod example running and ran into this, but if that's being used by others I'd assume this is hit elsewhere in the code base.

GAMMACASE commented 6 months ago

Hmm, how does that get set? It was my first night fiddling with getting a metamod example running and ran into this, but if that's being used by others I'd assume this is hit elsewhere in the code base.

Actually sorry, it was my local copy that had this added in the build script, so just assumed it was master as well. I guess it would be better to create a new pr with your changes and continue discussing there or move to discord?

inolen commented 6 months ago

What's the discord link?

GAMMACASE commented 6 months ago

What's the discord link?

https://discord.gg/HUc67zN

psychonic commented 6 months ago

@inolen we do have -Wno-sign-compare used in the build scripts, so I'm not sure if that needs fixing via the code or if we care enough for it? @psychonic would like to hear your opinion on that.

We've added warning suppression flags in the past, mostly due to existing Valve SDK code having problems. I would like to avoid introducing new warnings in contributed code if we can sanely do so.