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

GeoIP2 extension to sourcemod #1245

Merged
merged 46 commits into from
Jun 30, 2021
Merged

GeoIP2 extension to sourcemod #1245

merged 46 commits into from
Jun 30, 2021

Conversation

accelerator74
Copy link
Contributor

I updated the geoip extension for sourcemod using my geoip2 extension.

@accelerator74 accelerator74 changed the base branch from geoip2-wip to master April 26, 2020 11:56
Merge with the latest changes
When a player leaves during a voteban, he will be banned anyway. Also added a cvar with a ban time setting.
@Headline Headline added the Feature Request user requested feature label May 8, 2020
@KyleSanderson
Copy link
Member

@accelerator74 can you resolve the merge conflicts.
@psychonic do you know the latest on this stuff? I know it's been a minute since we looked at this stuff but you're way more present with the maxmind stuff than I am.

@peace-maker
Copy link
Member

Thank you! The fallback to the server language looks good.

I'll wait for a comment from other maintainers about the const char[] lang vs. int client parameter question to determine which language to use as proposed in my previous comment. Deliberately choosing a different language than the player's selected one to display a message to them sounds like a very "niche" use case. And you could still use SetClientLanguage briefly to force it to something else..

@accelerator74
Copy link
Contributor Author

Nevertheless, I made the definition of the language by the client index :) I see colleagues from the amxmodx team did the same :)

P.S.: By the way, yes, most of the code was borrowed from them, but it seems that everything is one team :)

@KyleSanderson
Copy link
Member

@peace-maker do you need anything from me to help move this one forward?

@peace-maker
Copy link
Member

@KyleSanderson No, this looks good. I didn't notice the changes after my last comment, so this is fine now from my side. If you don't have any strong feelings towards selecting the country language by string, 🚢
There are a couple of nits left like using .c_str() and then calculating the length again using strlen() while we could just use the length of the std::string and indentation issues in the include doc comments.

But above all this requires changes to the packaging scripts, since we can't ship the old GeoIp.dat with this anymore. We can't include the latest database either due to licensing requirements as you know. We can merge this and do a follow up to fix the packaging and documentation afterwards though, since that's not something Accelerator74 can do.

plugins/include/geoip.inc Outdated Show resolved Hide resolved
extensions/geoip/extension.cpp Outdated Show resolved Hide resolved
accelerator74 and others added 6 commits May 21, 2021 19:58
This package is the last CC-BY-SA licensed GeoLite2-City database extracted from https://src.fedoraproject.org/rpms/geolite2 from december 2019.

This doubles the download size for SM packages, but it's what we have to deal with atm :(
If the lookup failed, we'd copy back whatever is on the stack in the ccode buffer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request user requested feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants