Player class filter for maps

[DESTROYGIRL]

Description

A player class filter that grabs it from neo_player... please let me know if this sucks.

Toolchain

• Windows MSVC VS2022

Linked Issues

• resolves [FEATURE] Player class filter entity #655

[AdamTadeusz]

I don't know how this would be used, do you have a map where this is implemented?

Semantically I see no issues, I was thinking whether checking the entity index instead of calling IsPlayer would be cheaper since player entities are guaranteed an entity index between 1 and either gpGlobals->maxplayers or MAX_PLAYERS, but the other filters use IsPlayer so I wouldn't bother trying to optimize this particular bit.

One thing I did notice is 2 out of the 3 preexisting filter class names begin with a C and FilterNeoClass does not. Not sure whether thats important but it seems like this filter is more similar to the filters down below that have a C than the damage filter, but again probably not important

[DESTROYGIRL]

I don't know how this would be used, do you have a map where this is implemented?

Semantically I see no issues, I was thinking whether checking the entity index instead of calling IsPlayer would be cheaper since player entities are guaranteed an entity index between 1 and either gpGlobals->maxplayers or MAX_PLAYERS, but the other filters use IsPlayer so I wouldn't bother trying to optimize this particular bit.

One thing I did notice is 2 out of the 3 preexisting filter class names begin with a C and FilterNeoClass does not. Not sure whether thats important but it seems like this filter is more similar to the filters down below that have a C than the damage filter, but again probably not important

I believe the C at the beginning is just a naming scheme for stuff to represent that it's server side, I forgot about that. Not sure how to change it now I've made the pull request

This could be implemented in a variety of ways where you would want players of a specific class to activate something but not others. Like if you wanted only supports to take an elevator, or only recons be light enough not to break a bridge or something. This is nessecary functionality for the tutorial map I'm working on

[AdamTadeusz]

I don't know how this would be used, do you have a map where this is implemented?
Semantically I see no issues, I was thinking whether checking the entity index instead of calling IsPlayer would be cheaper since player entities are guaranteed an entity index between 1 and either gpGlobals->maxplayers or MAX_PLAYERS, but the other filters use IsPlayer so I wouldn't bother trying to optimize this particular bit.
One thing I did notice is 2 out of the 3 preexisting filter class names begin with a C and FilterNeoClass does not. Not sure whether thats important but it seems like this filter is more similar to the filters down below that have a C than the damage filter, but again probably not important

I believe the C at the beginning is just a naming scheme for stuff to represent that it's server side, I forgot about that. Not sure how to change it now I've made the pull request

This could be implemented in a variety of ways where you would want players of a specific class to activate something but not others. Like if you wanted only supports to take an elevator, or only recons be light enough not to break a bridge or something. This is nessecary functionality for the tutorial map I'm working on

Looks like you're working on your master branch, so any new commits you make locally to master that you then push will show up here. In the future consider making a separate branch for each new feature you are working on, that way you can work on multiple features at once

[Rainyan]

Thanks for the PR, left some feedback.

[Rainyan]

We've had the general convention of guarding NEO specific blocks of code inside Valve implementation with the:

#ifdef NEO
// Here's some NT code
#endif // NEO

similar to what Valve does with their games that do this kind of game-specific code block within the base engine code.

So since we are not inside a purely NT source code file here, this should be:

#ifdef NEO
#include "neo_player.h"
#endif

or similar.

Obviously we are only building for NT and not for other Source games, but abiding by this convention lets us locate NT-specific stuff in the code base more easily, by grepping #ifdef NEO etc.

[Rainyan]

Likewise, this class implementation should be guarded by a NEO ifdef.

[Rainyan]

Since the only kind of player we have is CNEO_Player, and we've already passed pEntity->IsPlayer to get here, we can safely assume this must be a CNEO_Player. Therefore, this can be a static_cast for performance. Or you may use the assert_cast, which will assert the dynamic_cast for debug builds, but optimize it into static_cast for release builds, if you want the guarantee nonetheless.

[Rainyan]

If using static_cast (or assert_cast) to assign the pPlayer, then it must be a valid non-null pointer here. So we can skip this check entirely.