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.

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

Add a flags update forward and a override param in set_user_flags #475

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

Add a flags update forward and a override param in set_user_flags #475

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8659c90
Add a flags update forward and a override param in set_user_flags
OciXCrom Dec 31, 2017
163b4e8
Update amxmodx.inc
OciXCrom Dec 31, 2017
e2c7ecb
Fixed misplaced param.
OciXCrom Jan 1, 2018
b4170b4
Fixed misplaced param
OciXCrom Jan 1, 2018
a3f5769
Added param availability check + 2 more params in the forward
OciXCrom Jan 2, 2018
6edebc9
Merge branch 'admin-flags-forward' of https://github.com/OciXCrom/amx…
OciXCrom Jan 2, 2018
9b26923
Added check for if the new flags are different
OciXCrom Jan 9, 2018
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
14 changes: 10 additions & 4 deletions amxmodx/amxmodx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2841,7 +2841,7 @@ static cell AMX_NATIVE_CALL get_user_flags(AMX *amx, cell *params) /* 2 param */
return GET_PLAYER_POINTER_I(index)->flags[id];
}

static cell AMX_NATIVE_CALL set_user_flags(AMX *amx, cell *params) /* 3 param */
static cell AMX_NATIVE_CALL set_user_flags(AMX *amx, cell *params) /* 4 param */
{
int index = params[1];

Expand All @@ -2860,8 +2860,13 @@ static cell AMX_NATIVE_CALL set_user_flags(AMX *amx, cell *params) /* 3 param */

if (id > 31)
id = 31;

pPlayer->flags[id] |= flag;

if(params[4])
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to check for parameter availability here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I'm fairly new to this. How do I exactly add that check?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example

amxmodx/amxmodx/amxmodx.cpp

Line 1443 in 7b3646a

if (params[0] / sizeof(cell) >= 12)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it actually necessary? The 4th parameter has a default value of "false" if it's not used. Just curious why it's needed.

Copy link
Contributor

@WPMGPRoSToTeMa WPMGPRoSToTeMa Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@OciXCrom you will need to recompile all plugins that using get_players.

Copy link
Contributor

Choose a reason for hiding this comment

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

@OciXCrom Plugins that don't get compiled against the new *.inc will pass you 3 parameters, and core needs to handle that situation gracefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Nextra @Amaroq7 @WPMGPRoSToTeMa - Thanks, I added the necessary check.

pPlayer->flags[id] = flag;
else
pPlayer->flags[id] |= flag;

executeForwards(FF_ClientFlagsUpdated, static_cast<cell>(pPlayer->index), pPlayer->flags[id]);

return 1;
}
Expand All @@ -2887,7 +2892,8 @@ static cell AMX_NATIVE_CALL remove_user_flags(AMX *amx, cell *params) /* 3 param
id = 31;

pPlayer->flags[id] &= ~flag;

executeForwards(FF_ClientFlagsUpdated, static_cast<cell>(pPlayer->index), pPlayer->flags[id]);

return 1;
}

Expand Down
1 change: 1 addition & 0 deletions amxmodx/amxmodx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ extern int FF_InconsistentFile;
extern int FF_ClientAuthorized;
extern int FF_ChangeLevel;
extern int FF_ClientConnectEx;
extern int FF_ClientFlagsUpdated;

extern bool g_coloredmenus;

Expand Down
2 changes: 2 additions & 0 deletions amxmodx/meta_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ int FF_InconsistentFile = -1;
int FF_ClientAuthorized = -1;
int FF_ChangeLevel = -1;
int FF_ClientConnectEx = -1;
int FF_ClientFlagsUpdated = -1;

IFileSystem* g_FileSystem;
HLTypeConversion TypeConversion;
Expand Down Expand Up @@ -516,6 +517,7 @@ int C_Spawn(edict_t *pent)
FF_ClientAuthorized = registerForward("client_authorized", ET_IGNORE, FP_CELL, FP_STRING, FP_DONE);
FF_ChangeLevel = registerForward("server_changelevel", ET_STOP, FP_STRING, FP_DONE);
FF_ClientConnectEx = registerForward("client_connectex", ET_STOP, FP_CELL, FP_STRING, FP_STRING, FP_ARRAY, FP_DONE);
FF_ClientFlagsUpdated = registerForward("client_flags_updated", ET_IGNORE, FP_CELL, FP_CELL, FP_DONE);

CoreCfg.OnAmxxInitialized();

Expand Down
13 changes: 12 additions & 1 deletion plugins/include/amxmodx.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,16 @@ forward client_connect(id);
*/
forward client_connectex(id, const name[], const ip[], reason[128]);

/**
* Called after the client's admin flags are changed.
*
* @param id Client index
* @param flags New client flags
*
* @noreturn
*/
forward client_flags_updated(id, flags);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd also pass id of set here, so we'll know which set of flags has been updated.

Copy link
Contributor Author

@OciXCrom OciXCrom Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I never knew different sets of flag existed until I saw the actual .cpp file. Are different sets ever used? Because if not, it would be rather unnecessary to have to write the set argument every time we use the forward if we'll never have to use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have to write all params in forward, they can be skipped, so if someone doesn't depend on sets, they can safely omit last param.

Copy link
Contributor

@rsKliPPy rsKliPPy Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I knew they existed but I never knew their purpose. The doc says:

AMXX stores multiple sets of flags internally, but only flag set
0 is actively used. You should not change the value of the third
parameter from the default.

I almost feel that we should just ignore flag sets and use a single one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if AMXX doesn't use other values than 0, it still may be used by 3rd plugins for their some specific permission system, so they don't mess up with the AMXX's one. I think including that param wouldn't do much harm, but it's just my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Amaroq7 we can add it later if someone really uses sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually if they can really be skipped when using the forwards (haven't actually checked this), then there's really no problem if that param can be passed too. I'll check this tomorrow and add it if that's the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Amaroq7 @rsKliPPy @WPMGPRoSToTeMa - I added the flag set as the last parameter in the forward, so people can skip it if not needed. I also passed the old flags so people can see which flags were changed.


/**
* Called when the client gets a valid SteamID.
*
Expand Down Expand Up @@ -1814,7 +1824,7 @@ native task_exists(id = 0, outside = 0);
* @error If the index is not within the range of 0 to MaxClients, an
* error will be thrown.
*/
native set_user_flags(index, flags = -1, id = 0);
native set_user_flags(index, flags = -1, id = 0, bool:override = false);

/**
* Returns the client's admin flags as a bitflag sum.
Expand All @@ -1826,6 +1836,7 @@ native set_user_flags(index, flags = -1, id = 0);
*
* @param index Client index, 0 to set flags of server
* @param id Flag set id, ranging from 0 to 31
* @param override If set to true, all other flags will be removed
Copy link
Contributor

Choose a reason for hiding this comment

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

Misplaced param's description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I always get confused when viewing .inc files because of the way the description is placed above the native. Fixed.

*
* @noreturn
* @error If the index is not within the range of 0 to MaxClients, an
Expand Down