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

Add 'j' and 'k' flags in get_players for matching with admin flags #476

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

OciXCrom
Copy link
Contributor

@OciXCrom OciXCrom commented Jan 1, 2018

"j" - match with all of the specified admin flags
"k" - match with any of the specified admin flags

@@ -2282,6 +2289,10 @@ static cell AMX_NATIVE_CALL get_players(AMX *amx, cell *params) /* 4 param */
continue;
if ((flags & 128) && (pPlayer->pEdict->v.flags & FL_PROXY))
continue;
if ((flags & 512) && ((pPlayer->flags[0] & pflags) != pflags))
continue;
if ((flags & 1024) && (!(pPlayer->flags[0] & pflags)))
Copy link

Choose a reason for hiding this comment

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

Maybe add consts for all flags?

@rsKliPPy
Copy link
Contributor

rsKliPPy commented Jan 1, 2018

You should also update get_players_ex to match this change.

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Jan 1, 2018

@rsKliPPy - I honestly didn't know that function even existed. Will do it tomorrow.

@rsKliPPy
Copy link
Contributor

rsKliPPy commented Jan 1, 2018

It's alright, it's a fairly recent addition.

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Jan 2, 2018

@rsKliPPy - done.

*
* @noreturn
*/
native get_players(players[MAX_PLAYERS], &num, const flags[] = "", const team[] = "");
native get_players(players[MAX_PLAYERS], &num, const flags[] = "", const string[] = "");
Copy link
Member

Choose a reason for hiding this comment

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

You can't rename params like that. You will break backward compatibility. People can still explicit the param .team = .

Copy link
Member

@Arkshine Arkshine Aug 17, 2018

Choose a reason for hiding this comment

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

I think it's okay to not update get_players anyway. We should encourage the use of flags as a string. We don't need to keep the same functionalities, after all, get_players_ex is an extended version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Arkshine I changed the param name back to team. I don't think however that get_players shouldn't be updated. get_players_ex is a stock that is depending on get_players.

stock get_players_ex(players[MAX_PLAYERS], &num, GetPlayersFlags:flags = GetPlayers_None, const team[] = "")
{
	new strFlags[12];
	get_flags(_:flags, strFlags, charsmax(strFlags));
	get_players(players, num, strFlags, team);
}

If we don't update get_players, this change will have no effect.

@Arkshine
Copy link
Member

It was fine to keep 'string' for get_players_ex, it was introduced in dev recently. If possible we should have a generic term here and actually something a bit more meaningful, maybe 'argument' or 'arg' ; don't know.

@OciXCrom
Copy link
Contributor Author

@Arkshine In my opinion it was fine for get_players too. I doubt that anyone will use parameter skipping here since all previous parameters in the function must be written in order for the team parameter to even be used. Simply said, get_players(iPlayers, iPnum, .team = "CT") is not valid, so chances are nobody will experience problems with backwards compatibility here.

Anyways, I'll change it back to string for get_players_ex if you want. I chose the name string because the description for the param says string to match against. I couldn't figure out a more appropriate name either.

@OciXCrom OciXCrom changed the title Add 'j' and 'k' flags in get_user_flags for matching with admin flags Add 'j' and 'k' flags in get_players for matching with admin flags Aug 17, 2018
@Arkshine
Copy link
Member

string is fine.

What do you mean by get_players(iPlayers, iPnum, .team = "CT") not being valid? It is valid.

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Aug 30, 2018

@Arkshine I put back string for get_players_ex.

I meant that it won't have an effect because if the "e" flag isn't specified then it won't search for the team at all so most likely people won't do that.

@WPMGPRoSToTeMa
Copy link
Contributor

WPMGPRoSToTeMa commented Sep 8, 2018

Arg name should also be fixed for get_playersnum_ex.
Also maybe we should replace &num argument in get_players_ex with return value?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants