-
-
Notifications
You must be signed in to change notification settings - Fork 288
feat: add max_line and max_lineHeight config settings #763
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
base: dev
Are you sure you want to change the base?
feat: add max_line and max_lineHeight config settings #763
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @xiaoxustudio, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此拉取请求旨在增强游戏内文本框的显示控制能力,通过引入Max_line
和Max_lineHeight
两个新的配置项,允许开发者更灵活地控制文本的显示行数和行高。这使得文本框的布局和可读性可以根据具体需求进行调整,从而提升用户体验。
Highlights
- 文本框行数限制: 引入了
Max_line
配置项,允许在config.txt
中设置文本框的最大显示行数。如果未设置,则使用默认逻辑。 - 文本框行高配置: 引入了
Max_lineHeight
配置项,当Max_line
启用时,可以自定义文本框的行高,单位为em
。 - 多行文本测试场景: 新增了
demo_max_line_test.txt
文件,用于测试和演示多行文本的显示效果。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个PR增加了Max_line
和Max_lineHeight
两个新的配置项来定制文本框,这是一个很好的功能增强。代码整体实现不错,但存在一些可以改进的地方。主要是在IMSSTextbox.tsx
中的行高计算逻辑存在一个bug,可能会导致显示异常。此外,两处修改都使用了不安全的类型转换,建议进行优化以提高代码的健壮性和可读性。我提供了一些具体的代码建议来修复这些问题。
const MaxTextLine = userDataState.globalGameVar.Max_line as number; // config定义字体行数 | ||
const MaxTextLineHeight = userDataState.globalGameVar.Max_lineHeight as number; // config 定义字体行数高度 | ||
const totalHeight = !Number.isNaN(Number(MaxTextLine)) | ||
? (Number.isNaN(Number(MaxTextLineHeight)) ? lineHeightValue : MaxTextLineHeight) * MaxTextLine | ||
: lineHeightValue; // Max_LineHeight和Max_Line必定为数字,否则使用默认值 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分代码存在几个问题:
- 不安全的类型转换:
MaxTextLine
和MaxTextLineHeight
使用as number
进行强制类型转换。由于这些值来自配置文件,它们可能是undefined
或非数字字符串,这种转换是不安全的。 - 误导性的变量名:变量
totalHeight
实际上用于计算行高(line-height
),而不是总高度,这会引起困惑。 - 错误的行高计算:在计算
line-height
时,将基础行高乘以MaxTextLine
(行数)是错误的。这会导致line-height
的值变得非常大,从而严重破坏文本布局。例如,如果MaxTextLine
为 4,lineHeightValue
为 2.2,line-height
会变成8.8em
,这显然不是预期的效果。
建议重构这部分逻辑,以确保类型安全和计算正确。
const lineHeightValue = textSizeState === textSize.medium ? 2.2 : 2;
const maxLine = Number(userDataState.globalGameVar.Max_line);
const maxLineHeight = Number(userDataState.globalGameVar.Max_lineHeight);
const finalLineHeight = !isNaN(maxLine)
? !isNaN(maxLineHeight)
? maxLineHeight
: lineHeightValue
: lineHeightValue;
const lineHeightCssStr = `line-height: ${finalLineHeight}em`;
const MaxTextLine = userDataState.globalGameVar.Max_line as number; // congfig定义字体行数 | ||
const lineLimit = Number.isNaN(Number(MaxTextLine)) | ||
? match(textSizeState) | ||
.with(textSize.small, () => 3) | ||
.with(textSize.medium, () => 2) | ||
.with(textSize.large, () => 2) | ||
.default(() => 2) | ||
: MaxTextLine; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分代码同样存在不安全的类型转换问题,并且可以写得更简洁清晰。
- 不安全的类型转换:
MaxTextLine
使用as number
强制转换,这存在风险。 - 潜在的类型错误:在三元表达式的
else
分支中,返回的是MaxTextLine
。如果移除as number
的强制转换,MaxTextLine
可能是字符串,而lineLimit
期望是number
类型,这可能导致类型不匹配问题。 - 代码可读性:
Number.isNaN(Number(MaxTextLine))
的写法有些冗余。 - 拼写错误:注释
// congfig定义字体行数
中congfig
应为config
。
建议进行如下修改以提高代码的健壮性和可读性。
const MaxTextLine = userDataState.globalGameVar.Max_line as number; // congfig定义字体行数 | |
const lineLimit = Number.isNaN(Number(MaxTextLine)) | |
? match(textSizeState) | |
.with(textSize.small, () => 3) | |
.with(textSize.medium, () => 2) | |
.with(textSize.large, () => 2) | |
.default(() => 2) | |
: MaxTextLine; | |
const maxLine = Number(userDataState.globalGameVar.Max_line); // config定义字体行数 | |
const lineLimit = !isNaN(maxLine) | |
? maxLine | |
: match(textSizeState) | |
.with(textSize.small, () => 3) | |
.with(textSize.medium, () => 2) | |
.with(textSize.large, () => 2) | |
.default(() => 2); |
增加
config.txt
配置:em
issue: #755