-
Notifications
You must be signed in to change notification settings - Fork 12
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
initial messages support #3
Conversation
copy from amxmodx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some small objections, but overall looks good.
If you also could add to readme in Acknowledgments
section that AMXX code has been used.
scripts/include/messages.inc
Outdated
#define MSG_PVS_R 6 // Reliable to PVS | ||
#define MSG_PAS_R 7 // Reliable to PAS | ||
#define MSG_ONE_UNRELIABLE 8 // Send to one client, but don't put in reliable stream, put in unreliable datagram (could be dropped) | ||
#define MSG_SPEC 9 // Sends to all spectator proxies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing spaces?
scripts/include/messages.inc
Outdated
|
||
native void MessageBegin(int dest, int msgid, const int origin[3] = {0,0,0}, int player = 0); | ||
|
||
native void MessageBegin(int dest, int msgid, const float origin[3] = {0,0,0}, int player = 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be MessageBeginF
?
const float origin[3] = {0,0,0}
-> const float origin[3] = {0.0,0.0,0.0}
to prevent tag mismatch.
src/MessageNatives.cpp
Outdated
{ | ||
enum { arg_dest = 1, arg_msg_type, arg_origin, arg_player }; | ||
|
||
int numparam = *params / sizeof(cell_t); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was correct in Pawn, this should be replaced with int numparam = *params;
src/MessageNatives.cpp
Outdated
#include "spmod.hpp" | ||
|
||
// int GetUserMsgId(const char[] msgname); | ||
static cell_t GetUserMsgId(SourcePawn::IPluginContext *ctx, const cell_t *params) /* 1 param */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to keep styling that every function parameter is in separate line.
static cell_t GetUserMsgId(SourcePawn::IPluginContext *ctx,
const cell_t *params) /* 1 param */
src/MessageNatives.cpp
Outdated
if (params[arg_msg_type] < 1 || ((params[arg_msg_type] > 63) // maximal number of engine messages | ||
&& !GET_USER_MSG_NAME(PLID, params[2], NULL))) | ||
{ | ||
// LogError(amx, AMX_ERR_NATIVE, "Plugin called message_begin with an invalid message id (%d).", params[2]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason that this and others LogError
weren't converted to ctx->ReportError
?
src/MessageNatives.cpp
Outdated
case MSG_ALL: | ||
case MSG_SPEC: | ||
case MSG_INIT: | ||
MESSAGE_BEGIN(params[arg_dest], params[arg_msg_type], NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nullptr
src/MessageNatives.cpp
Outdated
return 0; | ||
} | ||
|
||
MESSAGE_BEGIN(params[arg_dest], params[arg_msg_type], NULL, INDEXENT(params[arg_player])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nullptr
src/MessageNatives.cpp
Outdated
} | ||
|
||
// native void MessageEnd(); | ||
static cell_t MessageEnd(SourcePawn::IPluginContext *ctx, const cell_t *params) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing [[maybe_unused]]
.
In other functions as well.
src/MessageNatives.cpp
Outdated
{ | ||
enum { arg_msg = 1, arg_str, arg_len }; | ||
|
||
const char* string = GET_USER_MSG_NAME(PLID, params[arg_msg], NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nullptr
476da95
to
ed4bc66
Compare
LGTM. Thanks! |
|
||
native int GetUserMsgId(const char[] msg_name); | ||
|
||
native void GetUserMsgName(int msgid, char[] str, int len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here must be a return type int
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch, I overlooked it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in master.
copy from amxmodx
Description
Motivation and Context
How has this been tested?
Other cases difficult to test without own cmd registration
Screenshots (if appropriate):
Types of changes
Checklist: