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

Added whowas command #161

Closed
wants to merge 18 commits into from
Closed

Conversation

PlatinMTA
Copy link
Contributor

This should satisfice #116. This commit should add the command /whowas. The data is stored on the admin2 database. The table "whowas" on the database will store: playerName, lastIP, lastSerial and timeStamp.

Every time a player joins or quits the server the database will update. There could be many rows with the same playerName but it will certanly have a different serial. When using /whowas you will get all the information on the console.

Someone should check this commit before mergin it tho, I didn't find any problems but I also didn't test that much.

@Dezash
Copy link
Contributor

Dezash commented Jul 19, 2019

This creates a lot of unnecessary database queries that could affect server's performance. You could optimize the code significantly by caching data in a table and saving it to the database when the resource is stopped.

@PlatinMTA
Copy link
Contributor Author

This creates a lot of unnecessary database queries that could affect server's performance. You could optimize the code significantly by caching data in a table and saving it to the database when the resource is stopped.

Yes, you are right about the queries, but I don't really know how much of a performance issue this could cause and if having all that data on a table is even worth it. Just imagine having 10k rows on a single table and trying to search through it. Maybe I'm wrong and it goes ultra fast, I doubt it tho.

What I could do is have a recent user connection/disconnection table and save it when it reaches certain ammount of rows or when the resource stops, and then when we run the command it will go first through the table and if it doesn't find anything then it goes to the database. That should be a good solution.

qaisjp
qaisjp previously requested changes Jul 19, 2019
[admin]/admin2/server/admin_server.lua Outdated Show resolved Hide resolved
@Dezash
Copy link
Contributor

Dezash commented Jul 19, 2019

What I could do is have a recent user connection/disconnection table and save it when it reaches certain ammount of rows or when the resource stops, and then when we run the command it will go first through the table and if it doesn't find anything then it goes to the database.

Sounds good, you could also add settings to enable/disable the feature and to set the maximum amount of entries that it saves.

@PlatinMTA
Copy link
Contributor Author

Sounds good, you could also add settings to enable/disable the feature and to set the maximum amount of entries that it saves.

That's a good idea. I would do it tomorrow tho, it's like 5am here.

This adds:

aWhowas = {}
saveToDatabaseWhowas()
clearRowsWhowas()
insertIntoWhowas()

And updates:
updateWhowas()
This adds the ability to choose where do you want to print the info and makes it work with the changes to admin_server.lua
@PlatinMTA
Copy link
Contributor Author

Ok I just updated and made the table thing. You have three new settings now:

whowassave -> which selects which method you want to use for storing info
whowasmax -> which only works if whowassave is "both", it says which would be the maximum amount of rows into aWhowas
whowasprint -> which will set where should we print the info (if on console or chat).

Because saving into a table won't query the info on the database unless the table doesn't have the info we want, sometimes all the info won't be displayed. For that reason I choosed to set the default to "onlydb" on whowassave.

Other than that it should work just fine. You can search by name or by serial, depending of which you choose you will get more or less info.

As I stated before, it would be nice if someone could test this for a few minutes to see if something needs a fix.

@Dezash
Copy link
Contributor

Dezash commented Jul 20, 2019

Because saving into a table won't query the info on the database unless the table doesn't have the info we want, sometimes all the info won't be displayed.

Why not? Isn't the table saving the same data as database (nickname, serial, ip and last connection)? Which info won't be displayed?

@PlatinMTA
Copy link
Contributor Author

PlatinMTA commented Jul 20, 2019

Why not? Isn't the table saving the same data as database (nickname, serial, ip and last connection)? Which info won't be displayed?

The way I made it is that you can store multiple names and multiple serials whenever they both don't have the same info, for example you could have:

  1. Platin AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1
  2. Platin AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA2
  3. Platin_ AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1
  4. Platin_ AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA2

The problem with this is that maybe you have 1, 3 and 4 stored on the table, but you don't have 2 so whenever you run "/whowas Platin" you will get only 1, because it's in the table. Meanwhile if you use "/whowas Platin_" you will get 3 and 4 with no problem.

If someone joined the server it will always find it, no matter what, but sometimes it won't display all the information that is known from the user.

This is the problem I face when trying to save the recent ones on a table and the rest on the database. However it should improve on the number of queries executed on a semi-busy server and I don't think it really bothers anyone to have slightly less info about an user that just recently joined that day.

Do you have any other idea that I could go through so we don't have to deal with this issue?

EDIT: I could make it search the user on the table, store it, and then make it search it on the database. Maybe that's better? Because the mayor problem is when you execute the queries on saveToDatabaseWhowas()

@Dezash
Copy link
Contributor

Dezash commented Jul 20, 2019

I could make it search the user on the table, store it, and then make it search it on the database. Maybe that's better?

I think that would work. I also see an opportunity to improve updateWhowas by checking if any entries in aWhowas table match the new entry and update it instead of inserting a new one. It is better to do it on a table that contains ~500 entries than on the database that could contain millions.

@PlatinMTA
Copy link
Contributor Author

I think that would work. I also see an opportunity to improve updateWhowas by checking if any entries in aWhowas table match the new entry and update it instead of inserting a new one. It is better to do it on a table that contains ~500 entries than on the database that could contain millions.

I will try to do that in the following days.

@qaisjp
Copy link
Contributor

qaisjp commented Jul 21, 2019

I could make it search the user on the table, store it, and then make it search it on the database. Maybe that's better?

I think that would work. I also see an opportunity to improve updateWhowas by checking if any entries in aWhowas table match the new entry and update it instead of inserting a new one. It is better to do it on a table that contains ~500 entries than on the database that could contain millions.

didn't whowas give a history of instead of just the most recent field?

Deleted single line code and changed the dbQuery's calls to prevent SQL Injections
Deleted single line code, changed the dbQuery's calls to prevent SQL Injections, made it possible to use root on updateWhowas, changed updateWhowas() to persistPlayerWhowasInfo() and removed silly comments.
@PlatinMTA
Copy link
Contributor Author

PlatinMTA commented Aug 6, 2019

didn't whowas give a history of instead of just the most recent field?

I can't really recall correctly, I think it gave a history but not with all the information (like IPs sometimes wouldn't be store). Whoever this whowas also does that, but it has a flaw that I described before.

I just updated the commits and made the changes that were requested. If more changes need to be done please let me know, but I would probably take some days to fix them since I'm working on something for my server.

Also I do have the bad habit of making single line code, sorry about that.

I forgot about the singleline on the "whowasmax" check inside persistPlayerWhowasInfo()
@PlatinMTA
Copy link
Contributor Author

I just updated the branch so it's compatible with clearchat, didn't test it tho.

@qaisjp qaisjp removed their request for review April 3, 2020 12:08
@qaisjp qaisjp dismissed their stale review April 3, 2020 12:08

I guess that my requested changes have been applied

@PlatinMTA PlatinMTA closed this Oct 10, 2020
@Dutchman101
Copy link
Member

@PlatinMTA why did you close the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants