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

Enable unicode characters for TS channel and Arma server names #996

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tbeswick96
Copy link
Contributor

When merged this pull request will:

  • Allow unicode characters in TeamSpeak channel names and Arma server names
  • Change relevant TS3 methods for channel switching to use wstring instead of string
  • Add 2 conversion methods to convert between string and wstring
  • Stop the TextMessage __isascii check from running only for setTs3ChannelDetails procedure
  • Fix Non-ASCII characters break channel switching #579
  • Tested locally using a TeamSpeak channel name containing Grę and a local Arma MP instance with server name Grę. Channel switching works as expected. All other functionality seems unaffected (Could use a smoke test if anyone is interested in building and testing)

Notes:

  • The data coming from arma comes in as char*. This gets interpreted as ASCII, and the majority of data passed around inside ACRE is ASCII.
  • The unicode conversion for channel switching only happens in the RPC function setTs3ChannelDetails. This means data is sent into it in the "wrong" format, is converted and used accordingly, and where needed passed back out in the "wrong" format. The out I think is only for channel passwords. So it may be the case that a channel password containing unicode doesn't work as expected, however the bytes remain the same so TS should interpret it correctly. I've not tested this.

@jonpas jonpas added this to the 2.8.0 milestone Jul 21, 2020
@TheMagnetar
Copy link
Member

TheMagnetar commented Aug 8, 2020

What about using std::string test = u8"test" instead of wstring?
https://en.cppreference.com/w/cpp/language/string_literal

@tbeswick96
Copy link
Contributor Author

What about using std::string test = u8"test" instead of wstring?
en.cppreference.com/w/cpp/language/string_literal

u8, as far as i can see, can only be used for literals, so that doesn't help much for getting the string from a variable

Copy link
Member

@TheMagnetar TheMagnetar left a comment

Choose a reason for hiding this comment

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

Great stuff! only minor changes. The PR looks good but I would like to see how the mumble plugin supports it, before merging this branch into master.

extensions/src/ACRE2Shared/StringConversions.cpp Outdated Show resolved Hide resolved
extensions/src/ACRE2Shared/TextMessage.cpp Outdated Show resolved Hide resolved
@tbeswick96
Copy link
Contributor Author

@TheMagnetar Thanks, I don't have Mumble though so I can't test this with that, someone else with that all setup will need to lend a hand with that

@TheMagnetar
Copy link
Member

For sure, this is in our hands atm. Thanks for the PR @tbeswick96

@jonpas
Copy link
Member

jonpas commented Aug 14, 2020

Mumble compatibility branch here: https://github.com/IDI-Systems/acre2/tree/mumble-unicode

Tried with Test lığı as server name and it switched correctly on Mumble. However on TeamSpeak it moves me to the wrong channel (just uses first one that contains "ACRE" it seems). Could use another set of eyes.

@jonpas jonpas modified the milestones: 2.8.0, 2.9.0, 2.8.1 Sep 16, 2020
@jonpas
Copy link
Member

jonpas commented Feb 24, 2021

Did anyone get a chance to test this with my example? @tbeswick96

@jonpas jonpas modified the milestones: 2.8.1, 2.9.0 Feb 24, 2021
@tbeswick96
Copy link
Contributor Author

I've not forgotten about this, just really busy. Intending to try and get this done in a few weeks

@jonpas jonpas modified the milestones: 2.9.1, Backlog Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-ASCII characters break channel switching
3 participants