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

Rankings rewrite #474

Merged
merged 59 commits into from Oct 29, 2017
Merged

Rankings rewrite #474

merged 59 commits into from Oct 29, 2017

Conversation

shavitush
Copy link
Owner

@shavitush shavitush commented Aug 12, 2017

Fixes #465.

Note: This version of rankings doesn't support SQLite, I'm sorry but I can't wrap my head around getting UpdateAllPoints, UpdatePlayerPoints and RecalculateMap to work on SQLite. If you can do it, please send a pull request or post a patch here.

Differences:

  • This one is much MUCH MUCH faster.
  • Less database interaction.
  • No chat ranks here. They'll be re-added into shavit-chat after release. I don't have too much time for that right now unfortunately..
  • More caching. Your database won't die when people spam sm_top.
  • Points are calculated by tier.
  • Fixed SQL injection (thanks eXtaZy).
  • Removed Shavit_OnRankUpdated because it has no real use.

Extras:

  • Fixed {rand} in CS:GO chat processing.
  • Fixed back buttons in !replay.
  • Made zone calculation faster.
  • Remade key display! Now it'll be aesthetic for both CS:S and CS:GO. It'll also show players holding the JUMP button while on auto. Added it so players can help others that don't realize autobhop is on and instead keep scrolling. It's also decent to see if someone is cheating like that. I've also made it insta-update on key changes.
  • Fixed weapon_glock ammo not refilling.
  • Optimizing the whole database structure with indexes.

To-do before merging:

  • Add indexes where possible, redo column types. The index changes can't be nicely added, so I'll try my best to add documentation for how to manually add them when this is done! The changes will apply to newly created tables.
  • Fix an issue where !rank may show points of another player.
  • Fix an issue where times will sometimes not have points calculated.
  • Fix shavit-stats not taking tracks into consideration. Also do descending point sorting.
  • Fix [shavit-rankings.smx] Transaction query (0): Column 'points' cannot be null.
  • Fix WR menu showing rank 1/0 for bonus track.
  • Do a migration to both database/plugins so workshop/ will never show in map names.
  • Fix WR menu showing the map's id instead of the map's name when using !wr for workshop maps.
  • Remove head tilting in shavit-misc, when hiding.
  • Create the top 100 menu once, instead of many times.
  • Fix an issue where rankings only works properly when late-loaded.
  • Redo Shavit_GetDB (make it Shavit_GetDatabase).
  • Disallow sm_stats from opening before the menu is fully loaded.

To be done sometime:

After installing, set the map's tier with sm_settier and then run sm_recalcall (ROOT command) once to apply all the points retroactively. When you add new tiers for maps, the points for them will adjust accordingly.

I've tested this in a rather small server (10 users in the database) and everything was fine. Let me know if there are any issues. I'll merge when I'm done with implementing the last to-dos if there are no issues reported.

The forward won't be used at all and I don't plan on rewriting it, as
point recalculation will happens upon command/map change.
Considering I've added OnUserCmdPre this can be decent to help others
see that autobhop is enabled.

Added this because I was REALLY surprised that people don't seem to
realize you can hold the spacebar, and there are just so many of them..
it's insane.
This is what happens when I only test with USP :/.
@strafe
Copy link
Contributor

strafe commented Aug 12, 2017

Is there a default tier, if so is it set through a cvar? If not are maps just unranked by default or not playable?

@shavitush
Copy link
Owner Author

@strafe the default tier is 1. I decided to make the default tier 1 instead of anything higher because maps like bhop_freedompuppies are often forgotten and will award lots of points. Players will usually not report such issues to server admins as they benefit from the map being tiered high.

@shavitush
Copy link
Owner Author

@Lime18 my apologies for taking 18 days to investigate, but it should work now! :)
Let me know if there are any issues.

@Lime18
Copy link

Lime18 commented Sep 11, 2017

@shavitush It works, thanks :)

@Lime18
Copy link

Lime18 commented Sep 11, 2017

Could you add a command for scoring points all the maps? It's not very convenient to use sm_recalcmap on every map, when the maps are more than 200 :(
Or something simple, like sm_recalcmap mapname "sm_recalcmap bhop_lego2"

@shavitush
Copy link
Owner Author

@Lime18 will do!

Use the command (run as ROOT) to retroactively calculate points for all
maps, styles and tracks with tiers assigned.
@shavitush
Copy link
Owner Author

@Lime18 sm_recalcall is the new command to calculate all points.

@Lime18
Copy link

Lime18 commented Sep 12, 2017

@shavitush Thank you so much :)

Thanks to DoNotMiss from YOURGAME bhop server.
Default value of 'shavit_core_velocityteleport' has been changed to 0.
Change it manually in the config file of shavit-core for the changes to
apply.
Added `bool manual` to `Shavit_OnStyleChanged`.
I hope this won't be a bad solution, but I commented the code with a solution just in-case!
@shavitush
Copy link
Owner Author

Just wrote this for MySQL/MariaDB: this is a stored procedure that should replace SourceMod sending 1 query per ranked player. It works! However, the query takes over 50 seconds on my database with 10k~ users and 25k~ times, it's definitely not acceptable..
Does anyone have an idea for how this can be optimized? Obviously JOINing 2 times per iteration isn't the proper thing to do, but I can't figure out a better solution..

DROP PROCEDURE IF EXISTS UpdateAllPoints;
DELIMITER ;;
CREATE PROCEDURE UpdateAllPoints()
BEGIN
	DECLARE authid VARCHAR(32);
	DECLARE done INT DEFAULT 0;
	DECLARE cur CURSOR FOR (SELECT auth FROM playertimes WHERE points > 0.0 GROUP BY auth);
	DECLARE CONTINUE HANDLER FOR NOT FOUND SET done = 1;
	OPEN cur;
	ranks: LOOP
		FETCH cur INTO authid;
		IF done THEN
			LEAVE ranks;
		END IF;
		UPDATE users u JOIN (SELECT SUM((points * (@f := 0.975 * @f) / 0.975)) total FROM playertimes t
				CROSS JOIN (SELECT @f := 1.0) params WHERE points > 0.0 AND auth = authid ORDER BY points DESC) temp
				SET u.points = temp.total WHERE auth = authid;
	END LOOP;
	CLOSE cur;
END;;
DELIMITER ;
CALL UpdateAllPoints();

Got rid of a JOIN by directly setting the value. I'm still looking for a way to optimize this further, and this pull request sadly won't be merged until I do.
@shavitush
Copy link
Owner Author

Well.. using the procedure above, I decided to do some optimization with indexes/data types in my server's database. By doing that, I cut the execution time from 58.656 seconds to 5.750!

Before: /* Affected rows: 0  Found rows: 0  Warnings: 0  Duration for 3 queries: 58.656 sec. */
After: /* Affected rows: 0  Found rows: 0  Warnings: 0  Duration for 3 queries: 5.750 sec. */

I believe it can be optimized even further, but I'm not an SQL expert unfortunately so it's hard on me. I'll push a commit that adds the indexes and changes VARCHAR to CHAR(n) in the database. You'll have to do those changes on your own to your existing database in order to benefit from better performance.

@strafe
Copy link
Contributor

strafe commented Oct 29, 2017

Are you happy to merge this now @shavitush or still wanting to optimise it further?

@shavitush
Copy link
Owner Author

I'll implement the procedure into the code itself, and then I'll merge this pull request. Then I'll work on the other things in this to-do in another pull request.

@shavitush shavitush mentioned this pull request Oct 29, 2017
Ugly workaround but we can't use multiple queries in the same line in SourceMod.
@shavitush shavitush merged commit 9d40d05 into master Oct 29, 2017
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.

None yet

4 participants