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

Fix various compatibility issue with geoip module #198

Conversation

@Arkshine
Copy link
Member

commented Feb 6, 2015

  • After the big update (#99) It would seem I forgot to keep the expected return on failed lookup for geoip_country, which is supposed to be "error" - as documentation says it.
  • Considering the retardness of this above native, a new geoip_country_ex native is added (geoip_country is reverted to its original state)
  • Changed printf to MF_PrintSrvConsole to make sure messages appear in the console of a listen server.
  • Cleaned up a bit files.
@@ -117,6 +117,7 @@ static cell AMX_NATIVE_CALL amx_geoip_country(AMX *amx, cell *params)

if (!country)
{
MF_SetAmxString(amx, params[2], "error", params[3]); // to keep compatibility with previous version
return 0;

This comment has been minimized.

Copy link
@Nextra

Nextra Feb 6, 2015

Contributor

Incorrect return value for bcompat, the old version actually uses the MF_SetAmxString return value (and thus strlen("error")). I wouldn't consider this sensible behavior so maybe an _ex variant with a boolean proper return value is in order?

Also it looks like multiple function docs need to be fixed as some say that 0 will be returned on unsuccessful lookup which is incorrect.

@Arkshine Arkshine changed the title Fix a compatibility issue with geoip_country native Fix various compatibility issue with geoip module Feb 7, 2015

Arkshine
@Nextra

This comment has been minimized.

Copy link

commented on plugins/include/geoip.inc in 5241fdf Feb 7, 2015

fixit.png

@Nextra

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2015

Otherwise lgtm.

Arkshine
Arkshine pushed a commit that referenced this pull request Feb 7, 2015
Vincent Herbet
Merge pull request #198 from Arkshine/fix/geoip_country-return-on-fai…
…l-lookup

Fix various compatibility issue with geoip module

@Arkshine Arkshine merged commit 3e6d806 into alliedmodders:master Feb 7, 2015

0 of 2 checks passed

continuous-integration/appveyor Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci The Travis CI build is in progress
Details

@Arkshine Arkshine deleted the Arkshine:fix/geoip_country-return-on-fail-lookup branch Feb 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.