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

Replace SQL query for style map-top to prevent duplicate rows #284

Merged
merged 4 commits into from
Sep 5, 2021
Merged

Replace SQL query for style map-top to prevent duplicate rows #284

merged 4 commits into from
Sep 5, 2021

Conversation

1zc
Copy link
Member

@1zc 1zc commented Aug 18, 2021

Replacing the SQL query used in db_selectStyleMapTopSurfers to the same one used in db_selectMapTopSurfers. The old query brings up duplicate entries and a bunch of unnecessary rows by not searching for the right info.

Old query vs New query: (Sample used was surf_tendies on low-gravity with a total of 30 completions.)
image

Old query result: (First 15 rows)
image

New query result: (First 15 rows)
image

Copy link
Member

@Bara Bara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See no issues here, but would nice when someone, who use this, can confirm this "fix".

@1zc
Copy link
Member Author

1zc commented Aug 19, 2021

If someone else would like to test that would be awesome, but this is my testing from in-game low-gravity /mtop for surf_tendies on GFL.

Old:
image

New:
image

@DaLLuZZ
Copy link
Contributor

DaLLuZZ commented Aug 19, 2021

I just want to say that such a problem with mtop and srcp actually existed in the plugin. But this solution does not fully satisfy me. Why didn't you fix the query for srcp but only for mtop?
https://github.com/surftimer/Surftimer-Official/blob/3a492cc7ed4aa2ef23edcbb277c59969f721d9e2/addons/sourcemod/scripting/surftimer/sql.sp#L7318-L7321
I can also recommend you taking a look at the callback function. I think that you should delete some part of the code there. Maybe not in this PR, but keep in mind...
https://github.com/surftimer/Surftimer-Official/blob/3a492cc7ed4aa2ef23edcbb277c59969f721d9e2/addons/sourcemod/scripting/surftimer/sql.sp#L2116-L2122
I think you don't need to check if you have duplicate rows in result, because they are not possible due to primary key
https://github.com/surftimer/Surftimer-Official/blob/3a492cc7ed4aa2ef23edcbb277c59969f721d9e2/addons/sourcemod/scripting/surftimer/sql.sp#L148
Exactly the same should be done in sql_selectStageStyleTopSurfersCallback
https://github.com/surftimer/Surftimer-Official/blob/3a492cc7ed4aa2ef23edcbb277c59969f721d9e2/addons/sourcemod/scripting/surftimer/sql.sp#L7331

Copy link
Member

@Bara Bara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into the code, how about merging this query into sql_selectTopSurfers?

And as @DaLLuZZ wrote, you can and should remove the unnecessary code

@Bara Bara requested review from Bara and removed request for Bara August 19, 2021 13:35
@1zc 1zc requested a review from Bara September 4, 2021 07:51
@Bara
Copy link
Member

Bara commented Sep 4, 2021

Looks good for me, but test with approval would be better. 👌🏻

@Bara Bara merged commit 7cfb2ad into surftimer:master Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants