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

CLEO V2.0.0.7 #19

Closed
wants to merge 0 commits into from
Closed

CLEO V2.0.0.7 #19

wants to merge 0 commits into from

Conversation

XMDS
Copy link
Member

@XMDS XMDS commented Aug 25, 2023

fix #16

for(size_t i = 0; i < len; i++)
this->m_pText[i] = (unsigned char)text[i];
this->m_pText[len] = 0;
uint16_t* u16_text = new uint16_t[len + 1];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use wchar_t ?

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, it is better to use the uint16_t type, first of all it is more intuitive to tell us that it is 2 bytes (although the cleo library runs on win, but I have planned to port it to the mobile platform, the wchar_t type is not 2 bytes in some systems ), another reason is that my experience in Chinese games tells me that you should never use the wchar_t type to save Chinese characters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps then
typedef uint16_t wchar is most elegant solution.

this->m_pText[i] = (unsigned char)text[i];
this->m_pText[len] = 0;
uint16_t* u16_text = new uint16_t[len + 1];
memset(u16_text, 0, len + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Memset takes bytes count to set, so it will fill only half of the buffer. Maybe use std::fill instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I forgot, now it only has half the memory written with 0's.
Forgive me I prefer c style

@@ -24,6 +24,40 @@ CustomTextEntry::~CustomTextEntry()
delete m_pText;
}

static uint8_t get_utf8_bytes(uint8_t utf8)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should return regular number. Input type should be character. Function could be named better, like "get_utf8_character_size".
Parameter could be named "firstChar".

It would be nice to include comment with link:
https://en.wikipedia.org/wiki/UTF-8#Encoding

@@ -24,6 +24,40 @@ CustomTextEntry::~CustomTextEntry()
delete m_pText;
}

static uint8_t get_utf8_bytes(uint8_t utf8)
{
for (uint8_t i = 0; i < 6; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This project is using "opening bracket in new line" style. Better keep it consistent.

return 1;
}

uint16_t CustomText::convert_utf8_to_utf16(uint8_t* utf8, uint16_t utf8_len, uint16_t* utf16, uint16_t utf16_len)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use char\wchar_t for characters, int\unsigned int for counts.

bytes = get_utf8_bytes(utf8[i]);

if (bytes > 1) {
utf16[length] = utf8[i] & (0xFF >> (bytes + 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Many magic in single line. How about creating variable named "mask", then using in. Comments would be nice too.

for (uint8_t j = 1; j < bytes; j++) {
i++;
utf16[length] <<= 6;
utf16[length] += utf8[i] & 0x3F;
Copy link
Contributor

Choose a reason for hiding this comment

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

Magic numbers? 0x3F comes out of blue without any explanation.

if (bytes > 1) {
utf16[length] = utf8[i] & (0xFF >> (bytes + 1));
for (uint8_t j = 1; j < bytes; j++) {
i++;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can now i address out of data buffer?

Copy link
Member Author

@XMDS XMDS left a comment

Choose a reason for hiding this comment

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

You can change its name, although it doesn't matter

@MiranDMC
Copy link
Contributor

MiranDMC commented Oct 6, 2023

Please rebase on current master branch. We added some of similar changes already, so this PR will become simpler.

@XMDS XMDS closed this Nov 5, 2023
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.

关于cleo v1.1.1.7 可以支持无名汉化补丁的外接文本而 v2.0.0.6 不能外接中文的问题
2 participants