multitheftauto / mtasa-resources

This project maintains a list of up-to-date resources that come with Multi Theft Auto.
https://multitheftauto.com
MIT License
153 stars 157 forks source link

New `ip2c` resource #222

Closed qaisjp closed 1 year ago

qaisjp commented 4 years ago

Is your feature request related to a problem? Please describe.

The worktree gets dirty whenever we start the admin resource, because the admin resource occasionally generates a new IpToCountryCompact.csv. We can't delete that file (#221) because sometimes servers can't actually generate the file themselves (9e2005f05b007ceabbb63d2448f7735dd47dae57).

Other resources (the only non-default one is dxscoreboard) is unnecessarily dependent on admin to provide the country flags feature.

Describe the solution you'd like

TL;DR: We create a new ip2c resource without this data file in our repository. We generate the file when creating the main resources archive. dxscoreboard, admin, and admin2 will now auto-start ip2c if it exists, and function without country detection otherwise.

We can generate the IpToCountryCompact.csv file on the build server when creating our release archive—either by reimplementing the code in Python, or building some sort of Lua wrapper that implements some of MTA's native functions using publicly available rocks [0].

I'd prefer the latter solution instead of the former as then we don't need to duplicate the code or put effort into keeping it consistent. The question is how flexible the admin code currently is—hopefully this is not an issue as we just move ip2c to its own resource.

Instead of dxscoreboard and admin hard-failing when <include> can't find ip2c, we do some checks and manually start ip2c if it exists. Or we add an optional field to include instead of implementing the same logic everywhere. (Use case: what if the server author would like to keep ip2c loaded, but not have it auto-start when e.g. admin starts?)

Additional context

It would also be nice to use this as an excuse to GitHubify our build process here. It would be nice to move the mtasa-resources build task off the build server, and into GitHub Actions. /cc @ccw808

[0] Note that LuaRocks support will not be necessary inside MTA. It would only be used on the build server to generate the csv file.

qaisjp commented 4 years ago

Oh, looking at the code (why didn't I do this earlier?), admin just fetches the csv file from the MTA website. And I assume the MTA website just generates the csv using some script. It'd take just a couple seconds to add wget http://mirror.multitheftauto.com/mtasa/scripts/IpToCountryCompact.csv to the build steps.

See:

https://github.com/multitheftauto/mtasa-resources/blob/e51ed2fb1736c547ca0c7984d43a161d2c6872ac/%5Badmin%5D/admin/server/admin_ip2c.lua#L12-L14

ccw808 commented 4 years ago

It would be nice to move the mtasa-resources build task off the build server, and into GitHub Actions.

Good idea

jlillis commented 4 years ago

To complicate things, admin2 uses an entirely different approach from admin. It ships with a bunch of XML files that have no update mechanism: https://github.com/multitheftauto/mtasa-resources/blob/20b75b74e8ef0fc2ff4687f51b27e13bf04736a6/%5Badmin%5D/admin2/server/admin_ip2c.lua#L183-L206

To simplify things, perhaps we could: 1: Create ip2c as a wrapper for some free and publicly-available web API (ip2c.org would be a good candidate). This negates the need for any ip2c-related infrastructure on MTA HQ's side. 2: Refactor the ip2c functionality in admin, admin2, and scoreboard to use the ip2c resource's export functions

If we wait until after admin2 reaches feature parity with admin (which is not very far) we can skip refactoring admin and just throw IpToCountryCompact.csv into .gitignore for now.

ccw808 commented 4 years ago

I don't think having a dependency on a third party web site is a good idea for the official resources. We have no control over what may happen to it in the future. Having a dependency on MTA infrastructure is preferred because servers already rely on it.

jlillis commented 4 years ago

I had some concerns about using a third-party at first too, but I think in this instance it is acceptable because:

  1. This particular third party appears to be pretty reliable. From their about page:

    This service is FREE (LGPL) because we strongly support technology sharing. FREE does not imply low quality. Our uptime has been 99.95% since the beginning of 2009

  2. IP geolocation isn't (or shouldn't be) considered a critical feature, so if the provider goes down or returns inaccurate results isn't a big deal.

  3. We can always issue an API-compatible update later on to switch providers (including one hosted by MTA HQ) if needed in the future.

ccw808 commented 4 years ago

We can't issue a resource update. If the chosen site goes down or changes its API then it will affect all MTA servers using the ip2c resource

Pirulax commented 4 years ago

My naive idea is to route it thru MTA, and if the API goes down, then we can change it to a new one?

Woovie commented 4 years ago

We can do this similarly to mtasa.com/api/. We could generate the XMLs on a cron.

ccw808 commented 4 years ago

My naive idea is to route it thru MTA, and if the API goes down, then we can change it to a new one?

That's what sort of happens now. I don't see what is wrong with the current system

qaisjp commented 4 years ago

Yes, I agree we should stick with MTA infrastructure.

Next step would be to investigate the difference between admin and admin2's IPC system and figure out why admin2 does something different, and what the benefits are. Then it's worth spending the time to update the MTA backend to deliver xml files instead of a single csv.

(Sorry for the delay, I kept typing this up and losing the text in browser reboots.)

jlillis commented 4 years ago

I've created a PR that resolves the initial concern about the worktree. In the long term a separate ip2c resource is ideal/GitHub Actions setup is ideal. I understand the desire to stick with MTA's infrastructure. Which approach do we want to use though? admin's IpToCountryCompact.csv or admin2's XML files? I'm inclined to just stick with admin's approach since we know it works well.

qaisjp commented 4 years ago

I'm inclined to just stick with admin's approach since we know it works well.

Sounds reasonable. We could leave admin2 alone and investigate its new approach later.

Dutchman101 commented 4 years ago

If there is someone who wants to do it, I don't think there is a reason to do that later.. my opinion is that admin's approach is the best, and admin2 is just outdated with plenty of things, including its approach to ip2c. Using .csv files under our current infrastructure is our best bet to keep country definitions updated, you can probably reuse some code from the admin ip2c auto-updater as well.

qaisjp commented 4 years ago

The reason I say "investigate its new approach later" is because:

xLuxy commented 4 years ago

I looked around the differences in admin and admin2 and here are the differences i found:

besides that, admin2's code seems to be cleaner than admin.

jlillis commented 3 years ago

Does admin2 not have an update mechanism for it's xml files?

qaisjp commented 3 years ago

Does admin2 not have an update mechanism for it's xml files?

Nope that functionality was added at a later point to admin1 (by ccw)

jlillis commented 1 year ago

Resolved via #405