search

Aireil / FFLogsViewer

Dalamud plugin to view FF Logs ranking percentiles in-game
47 stars 16 forks source link

Add support for CN region #11

Closed Nukoooo closed 2 years ago

Nukoooo commented 2 years ago

Code is bad because I am still pretty new to C# and due to my implementation it will have a false positive when searching player with clipboard text

Aireil commented 2 years ago

Hey, thanks for the PR, just two questions. Has this been tested on the Chinese client? Could you give me a few examples of full Chinese names including world?

Nukoooo commented 2 years ago

Has this been tested on the Chinese client?

Yes

Could you give me a few examples of full Chinese names including world?

青砖伴瓦漆@海猫茶屋 / 卡比man@紫水栈桥 / 朝日奈五月雨@延夏 And this will give you a false positive if you serach from clipboard text ( like joining someone's party ) 加入了猫小奶@紫水栈桥的小队。

Aireil commented 2 years ago

Ah yes, that seems harder to deal with automatically. My only issue is the DC region of 0 matching CN, because it's associated to INVALID and Beta in the global client. I'm asking Blue about it, if there is no alternative, I will just add a different check when on the CN client.

Aireil commented 2 years ago

Do placeholders work as well? If you try /fflogs <t> with a player targeted.

Nukoooo commented 2 years ago

Yes it works

Aireil commented 2 years ago

OK, turns out the Chinese client handles the worlds differently. I went ahead and added a check based on the client language. Could you test the latest commit on the Chinese client and see if it still works please? I also adjusted the UI as it seems like it doesn't use the last name, tell me if that is incorrect. 1GVuGKv

Nukoooo commented 2 years ago

Can confirm the latest commit works, and the UI looks good to me! Also I just found a mistake I made while testing the change. https://github.com/Aireil/FFLogsViewer/blob/829c889bc81a3fba30137c529c1d91ba38aa2748/Plugin.cs#L279 This line should be fine without substring-ing considering it has a world name check.

Also a little suggestion here: https://github.com/Aireil/FFLogsViewer/blob/829c889bc81a3fba30137c529c1d91ba38aa2748/Plugin.cs#L366 Changing the error message to something like "Character is not found on FF Logs" would be less confusing ( Yes I got confused the first time when I saw this message )

Aireil commented 2 years ago

Perfect, just updated the repo, should be available in a few minutes (version 1.5.7.0). Thanks for the help 👍.