Skip to content
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.

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

Merge MP branch into SP #378

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

Blixibon
Copy link
Member

@Blixibon Blixibon commented Feb 1, 2025

This PR merges all SDK code changes in the Source 2013 MP branch into the Source 2013 SP branch. Ideally, this will put code in the sp and mp folders into a position where they can be partially synchronized. Changes which cannot be ported due to engine interfaces, incompatible specifications, etc. are gated behind a new SDK_MP preprocessor.

Most of these changes are very small or are limited to areas that are not commonly used, although the number of changes alone could potentially have far-reaching consequences.

I started working on this for two reasons:

  1. This will make it much easier to create and maintain a MP port, as any changed files in one branch can be directly copied to the other. However, I would still need to figure out how to enforce this easily and fairly.
  2. To port any remaining enhancements in the MP branch to the SP branch, as the MP branch reflects a newer version of the SDK codebase. For example, we recently ported a new point_spotlight keyvalue from the MP branch, although I don't think there's any other enhancements on a similar level which we haven't ported already.

This PR is a draft for many reasons. I have been able to build and test this locally, but I have yet to thoroughly test the compatibility of this branch with other mods, or on Linux.

If you have any input or objections to this PR, whether that's with specific parts of this implementation or with the idea as a whole, then please feel free to comment or reach out to me directly.

Note that this pull request will be squashed when it is merged.


PR Checklist

  • My PR follows all guidelines in the CONTRIBUTING.md file
  • My PR targets a develop branch OR targets another branch with a specific goal in mind

@Blixibon Blixibon added the MP Branch Mapbase - Related to/held off until MP branch label Feb 1, 2025
@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from 0692992 to d20762c Compare February 2, 2025 18:20
@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from d20762c to be96c2b Compare February 2, 2025 18:47
@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from f342980 to 8cae4f3 Compare February 4, 2025 07:14
@Yankumo-Koishi
Copy link

What was I thinking is merge Mapbase to MP Branch since it has more few features (due the engine differences)
But uh that's probably too difficult so this one is enough

@Blixibon
Copy link
Member Author

Blixibon commented Feb 4, 2025

What was I thinking is merge Mapbase to MP Branch since it has more few features (due the engine differences) But uh that's probably too difficult so this one is enough

Mapbase is still going to be ported to the MP branch. This PR is just a way of getting to that point.

By synchronizing files between the SP and MP branches, their code can be kept in sync. Changes made in one branch can be directly copied to the other. This would (ideally) make it easier to maintain both branches at once.

@Blixibon
Copy link
Member Author

Blixibon commented Feb 4, 2025

Since opening this PR, I've decided to limit the scope of this merge.

Most of the gated changes in engine interfaces are redundant because those files have no reason to change under Mapbase, and thus have no reason to be synchronized. This means changes to all files in the src/public folder have been reverted except a SP-specific fallback case in tier0/platform.h.

For now, only the following directories have had code ported from the MP branch and may be synchronized with their MP branch counterparts:

  • src/game (client and server)
  • src/materialsystem/stdshaders (shaders)
  • src/utils (compile tools and beyond)

Mapbase-specific files which do not exist in vanilla MP (e.g. the vscript and responserules libraries, Mapbase's tier1 files) may be synchronized as well.

In addition, I've gated a bunch of other changes in the game code to become MP-only. I used the following criteria to determine which changes to exclude:

  1. It is not immediately understood what the change does.
  2. It is not immediately understood why the change was made.
  3. The change would require mods/derived classes to conform to something. (e.g. the activity list syntax change in CBaseCombatWeapon)
  4. The change is within a static library and it is exposed to other libraries. (e.g. changes within headers of the src/public folder)
  5. The change is within an interface or abstract class of any kind. (classes with = 0 pure virtual functions)
    • Classes which override interface/abstract classes do not count for this as long as they are contained to their library.

During this process, I did a complete rereview of all of this PR's changes, and I kept track of notable changes which I've decided to retain. Here's a preliminary changelog separated by code project:

Client

  • Fix animated texture proxy retaining material var pointers after failing to initialize.
  • Fix jiggly bones being shown incorrectly when parent bones are scaled by transforms.
  • Add capability for client-only CBaseCombatCharacter glow effect (until now, glow effect could only be enabled by the server)
  • New r_drawtracers and r_drawtracers_firstperson cvars for controlling tracer visibility.
  • Fix certain VGUI panels not being deleted properly.
  • Add description to viewmodel_fov cvar.

Server

  • SetModelScale's time parameter is now much smoother. You can use the time parameter by specifying another number after the scale (e.g. a parameter of "2 5" will scale the model 2x over the course of 5 seconds). This is an existing feature in Source 2013 Singleplayer, but it had limited uses because it previously scaled the model in rigid steps.
  • New sv_allow_point_servercommand cvar to control whether point_clientcommand can be used.
  • Fix certain npc_metropolice stitching values using ints instead of floats.
  • Add support for pose parameter override tables on weapons.

Shared

  • Add support for "velocity" keyvalue in prop gibs.

Shaders

  • Add SDK_DebugLuxel
  • Add support for $phongexponentfactor on SDK_VertexLitGeneric
  • Nonfunctional support for lightmaps on SDK_VertexLitGeneric. This is noted because the compiled shaders will work one-to-one between the SP and MP branches, so the shader code is included, but the actual lightmap data requires MP engine libraries.

VBSP

  • New -embed option which can embed a directory into the BSP.

A more complete and revised version of this will probably appear in whatever update includes these changes.

@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from d5e4807 to 44d5c94 Compare February 4, 2025 22:04
Copy link
Member Author

@Blixibon Blixibon left a comment

Choose a reason for hiding this comment

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

I left a bunch of comments to help aid potential reviewers.

For reference, many changes are gated within preprocessors for TF2 and/or STAGING_ONLY. These changes can be ignored. There are also many changes to multiplayer-exclusive files (e.g. hud_vote.cpp) that I did not review as thoroughly or explore legacy support for due to their presumed lack of use in SP branch mods.

Comment on lines -42 to +72
if( !pAnimatedTextureVarName )
return false;

bool foundVar;
m_AnimatedTextureVar = pMaterial->FindVar( pAnimatedTextureVarName, &foundVar, false );
if( !foundVar )
return false;

char const* pAnimatedTextureFrameNumVarName = pKeyValues->GetString( "animatedTextureFrameNumVar" );
if( !pAnimatedTextureFrameNumVarName )
return false;
if( pAnimatedTextureVarName )
{
bool foundVar;

m_AnimatedTextureFrameNumVar = pMaterial->FindVar( pAnimatedTextureFrameNumVarName, &foundVar, false );
if( !foundVar )
return false;
m_AnimatedTextureVar = pMaterial->FindVar( pAnimatedTextureVarName, &foundVar, false );
if( foundVar )
{
char const* pAnimatedTextureFrameNumVarName = pKeyValues->GetString( "animatedTextureFrameNumVar" );

if( pAnimatedTextureFrameNumVarName )
{
m_AnimatedTextureFrameNumVar = pMaterial->FindVar( pAnimatedTextureFrameNumVarName, &foundVar, false );

if( foundVar )
{
m_FrameRate = pKeyValues->GetFloat( "animatedTextureFrameRate", 15 );
m_WrapAnimation = !pKeyValues->GetInt( "animationNoWrap", 0 );
return true;
}
}
}
}

m_FrameRate = pKeyValues->GetFloat( "animatedTextureFrameRate", 15 );
m_WrapAnimation = !pKeyValues->GetInt( "animationNoWrap", 0 );
return true;
// Error - null out pointers.
Cleanup();
return false;
Copy link
Member Author

Choose a reason for hiding this comment

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

The diff isn't very clear on this, but this basically consolidates all fail cases into one that calls Cleanup(), which removes the pointers for material vars. This apparently happens if some vars are valid and obtained, but others are not, causing the proxy to retain material vars despite failing to initialize.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file and its header do not appear to be used in the MP branch, and I suspect that it's used in TF2's workshop code. Perhaps it could be removed from SP and be excepted from any potential synchronization script/hook?

@@ -153,9 +153,13 @@ void C_AI_BaseNPC::OnDataChanged( DataUpdateType_t type )
}
}

void C_AI_BaseNPC::GetRagdollInitBoneArrays( matrix3x4_t *pDeltaBones0, matrix3x4_t *pDeltaBones1, matrix3x4_t *pCurrentBones, float boneDt )
bool C_AI_BaseNPC::GetRagdollInitBoneArrays( matrix3x4_t *pDeltaBones0, matrix3x4_t *pDeltaBones1, matrix3x4_t *pCurrentBones, float boneDt )
Copy link
Member Author

Choose a reason for hiding this comment

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

I have a comment regarding this function in c_baseanimating.h.

void ForceSetupBonesAtTime( matrix3x4_t *pBonesOut, float flTime );
virtual void GetRagdollInitBoneArrays( matrix3x4_t *pDeltaBones0, matrix3x4_t *pDeltaBones1, matrix3x4_t *pCurrentBones, float boneDt );
bool ForceSetupBonesAtTime( matrix3x4_t *pBonesOut, float flTime );
virtual bool GetRagdollInitBoneArrays( matrix3x4_t *pDeltaBones0, matrix3x4_t *pDeltaBones1, matrix3x4_t *pCurrentBones, float boneDt );
Copy link
Member Author

Choose a reason for hiding this comment

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

I have mixed feelings about this because it changes the return value of a virtual function. If there are any SP mod classes which override this function, then they will have to adapt.

I decided to leave it as-is for the following reasons:

  1. Mods are unlikely to override GetRagdollInitBoneArrays.
  2. Gating every instance of this change being utilized would be time-consuming.

I'm open to input on this.

Comment on lines -1481 to -1492
//-----------------------------------------------------------------------------
// Returns the health fraction
//-----------------------------------------------------------------------------
float C_BaseEntity::HealthFraction() const
{
if (GetMaxHealth() == 0)
return 1.0f;

float flFraction = (float)GetHealth() / (float)GetMaxHealth();
flFraction = clamp( flFraction, 0.0f, 1.0f );
return flFraction;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is moved to baseentity_shared.cpp.

@@ -194,7 +194,6 @@ class CGlobalEntityList : public CBaseEntityList

CBaseEntity *FindEntityNearestFacing( const Vector &origin, const Vector &facing, float threshold);
CBaseEntity *FindEntityClassNearestFacing( const Vector &origin, const Vector &facing, float threshold, char *classname);
CBaseEntity *FindEntityByNetname( CBaseEntity *pStartEntity, const char *szModelName );
Copy link
Member Author

Choose a reason for hiding this comment

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

This function did not have a body and was not used by any other code.

Copy link
Member Author

Choose a reason for hiding this comment

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

The files prefixed with nav are used for navmeshes, which are not used in default HL2 or Mapbase.

Copy link
Member Author

Choose a reason for hiding this comment

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

The diff in this file is a little hard to read, but the change to NPC interjections appears to only be a refactor and they function identically to before.

Comment on lines +2982 to +3007
//-----------------------------------------------------------------------------
// Purpose:
//-----------------------------------------------------------------------------
void CBaseCombatWeapon::PoseParameterOverride( bool bReset )
{
CBaseCombatCharacter *pOwner = GetOwner();
if ( !pOwner )
return;

CStudioHdr *pStudioHdr = pOwner->GetModelPtr();
if ( !pStudioHdr )
return;

int iCount = 0;
poseparamtable_t *pPoseParamList = PoseParamList( iCount );
if ( pPoseParamList )
{
for ( int i=0; i<iCount; ++i )
{
int iPoseParam = pOwner->LookupPoseParameter( pStudioHdr, pPoseParamList[i].pszName );

if ( iPoseParam != -1 )
pOwner->SetPoseParameter( iPoseParam, bReset ? 0 : pPoseParamList[i].flValue );
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Pose parameter overrides seem to be a new type of table weapons can define, similar to activity lists. I have not tested this feature, but I've decided to leave it in.

#ifdef SDK_MP
virtual void ReloadScheme( bool flushLowLevel );
#else
virtual void ReloadScheme( bool flushLowLevel = false );
Copy link
Member Author

Choose a reason for hiding this comment

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

flushLowLevel is non-functional in SP, but the parameter is still implemented so that code which calls the function can be one-to-one between SP and MP.

@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from 1e527f1 to 69d7747 Compare February 6, 2025 17:41
@Blixibon Blixibon force-pushed the mapbase/feature/mp/sp-with-mp-changes branch from 69d7747 to 3e2e4cd Compare February 6, 2025 18:12
@Blixibon Blixibon marked this pull request as ready for review February 7, 2025 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MP Branch Mapbase - Related to/held off until MP branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants