alliedmodders / amxmodx

AMX Mod X - Half-Life 1 Scripting and Administration
http://www.amxmodx.org/
494 stars 197 forks source link

Names of extended/updated functions #600

Open Hembi opened 6 years ago

Hembi commented 6 years ago

Hi,

I have just reading the API Changes description of the new version of AmxModX and I can not agree with these new chaotic function names. The best part to give an example is the "Existing core APIs additions": client_connectex (why not just client_connected?) client_disconnected (may this one is correct, because the client_disconnect is deprecated, but this is also an extended function) below are these ones: find_player_ex get_players_ex get_playersnumex In this case why they got an "" mark before the extended mark (or client_connectex why not?)?

It would be nice if would be a rule for these new names. If a function has an extended version much easier to find it if the new function names follow a rule.

WPMGPRoSToTeMa commented 6 years ago

Ye, client_disconnected name is a bit strange, it looks like client_disconnect_post, but really it is advanced client_disconnect which is called even when player disconnects in connecting state. So I think it should be client_disconnect_ex or something like this. There is also client_remove which should be named actually client_disconnected_ex or client_disconnect_ex_post, because it is called after client_disconnected. Also we can choose drop for it, because the engine function is SV_DropClient. So, client_disconnected should become client_drop and client_remove should become client_dropped or client_drop_post.

Arkshine commented 6 years ago

client_disconnected was my mistake. If anything it should have been called client_disconnecting. I don't agree to prefix with _ex, this is not an extended version, it's a drop-in replacement. As for client_remove, it should have been called client_disconnected. But it's too late now. Well, please be indulgent, apart Nextra, no one ever tried to help before, we just lacked feedbacks, the struggle was real. If you don't want this to happen again, please contribute.

WPMGPRoSToTeMa commented 6 years ago

@Arkshine we can add aliases for them (client_drop and client_dropped or something like that).

Hembi commented 6 years ago

I would like to help your work with some feedbacks, may be later in the coding, but I have limited time now. I think the client_disconnecting and client_disconnected would be nice for these functions (and the client_drop and client_dropped aliases also), but I have to note it again a rule would be nice for these names in the 1.9.0 of AmxModX, we have to discuss about this and may make a vote about the best suggestions. I do not know what's the future plan after release the stable 1.9.0, but: The _ex postfix would be nice when the 1.10 will be continuing we can just left deprecated functions, delete the _ex postfix and use the original name of the functions. In the plugins which use the _ex functions can be easily correct with a search&replace.

PartialCloning commented 6 years ago

You're making it out to be a much bigger deal than it is. There are three forwards. client_disconnect, it's deprecated, you'll get a warning if you use it. It's not something you have to worry about.

That leaves us with two forwards. client_disconnected: The client disconnected, networked commands won't reach the client but the entity is still valid. client_remove: Called when the entity is removed and is no longer valid, after the client disconnected.

The current names are acceptable and fit the description. Adding aliases will make it confusing.

WPMGPRoSToTeMa commented 6 years ago

@PartialCloning then client_remove should be client_removed, isn't it?

Hembi commented 5 years ago

@PartialCloning I only gave some examples in my first comment and not only these functions what I was thinking about. In the 1.9 we got some other extended functions which are named with different suffix, some of them are without completeness: register_event_ex client_printex replace_stringex strtok2 (but this bugfixed version of strtok)

Yeah, I know that this is not a big deal, but we can discuss about these names to provide more consistent names to the plugin developers which can improve the experince of developing.

Arkshine commented 5 years ago

It's too late to care about global consistency. What we can do is only trying to be consistent with the context. But I guess we could say to use "2" for a fixed version and "_ex" for an extended version.

OciXCrom commented 5 years ago

Why not simply fix the existing native instead of adding a new one marked with "2"?

Arkshine commented 5 years ago

It was implied, but only if there is a behavior change or can't be reasonably fixed in the original native.