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

Fix charset #536

Merged
merged 2 commits into from
Nov 23, 2018
Merged

Fix charset #536

merged 2 commits into from
Nov 23, 2018

Conversation

Groruk
Copy link
Member

@Groruk Groruk commented Nov 23, 2018

Close #506
Close #505
Fix #236
Fix #390

Description

SQL_SetCharset seems to be bugged and defaults to latin1 as a charset. To fix this we are now using "SET NAME" queries exclusively (until a fix for SQL_SetCharset is found)

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

SQL_SetCharset seems to be bugged and defaults to latin1
@Groruk Groruk changed the title Fix charset defaulting to latin1 Fix charset Nov 23, 2018
@Groruk Groruk merged commit 14b887e into sbpp:v1.x Nov 23, 2018
@Groruk Groruk deleted the fix-charset branch November 23, 2018 00:20
@CrazyHackGUT
Copy link
Contributor

This is very dumb fix ever i seen.
MySQL extension can't escape characters properly, if character set is changed with simple query.
You need think about this "fix" and revert PR until someone started having problems.

@Bara
Copy link

Bara commented Nov 23, 2018

And Sourcemod 1.9 and lower don't support utf8mb4.
image
Here's the link to the message:
https://discordapp.com/channels/335290997317697536/335290997317697536/504723911854915585

@CrazyHackGUT
Copy link
Contributor

Maybe instead of a hasty merging of PR, you give some time for another users for testing?
I don't see any reasons for using PR, if no one can't review their before merging.

Think about it, @Groruk. And, please, stop merging PR without reviews.

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.

Bugs in the nick with characters Comms and Bans Weird Character Problem
3 participants