-
-
Notifications
You must be signed in to change notification settings - Fork 72
feat: support json game config #510
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?
Conversation
Summary of ChangesHello @HardyNLee, 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! 本次拉取请求旨在通过引入 JSON 格式的游戏配置文件来现代化游戏配置管理,同时确保与现有 TXT 格式的配置文件保持完全兼容。这一改进不仅提升了配置文件的可读性和可维护性,还为未来扩展配置项提供了更灵活的基础。通过统一的 'IWebgalConfig' 接口处理配置数据,无论底层存储格式如何,都能提供一致的开发体验。 Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
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 引入了对 JSON 格式游戏配置的支持,并保持了对旧版 .txt 格式的兼容性,这是一项重要的重构。通过引入 IWebgalConfig 接口和转换工具函数,代码的结构化和可维护性得到了提升。
我的审查主要关注以下几点:
- 健壮性: 后端 API 在处理文件不存在或格式错误时的容错能力。
- 安全性: 对来自前端的数据进行安全的解析。
- 代码质量: 前端组件中的代码重复问题和一些可以改进的实现细节。
总体来说,这是一个很好的功能增强,我的建议旨在进一步提高代码的稳定性和可维护性。
| async getGameConfig(@Param('gameName') gameName: string) { | ||
| const configFilePath = this.webgalFs.getPathFromRoot( | ||
| `/public/games/${decodeURI(gameName)}/game/config.txt`, | ||
| const isLegacyConfig = await this.webgalFs.exists( | ||
| this.webgalFs.getPathFromRoot( | ||
| `/public/games/${decodeURI(gameName)}/game/config.txt`, | ||
| ), | ||
| ); | ||
| return this.webgalFs.readTextFile(configFilePath); | ||
| if (isLegacyConfig) { | ||
| const configFilePath = this.webgalFs.getPathFromRoot( | ||
| `/public/games/${decodeURI(gameName)}/game/config.txt`, | ||
| ); | ||
| const textConfig = await this.webgalFs.readTextFile(configFilePath); | ||
| if (typeof textConfig === 'string') { | ||
| const webgalConfig = convertTextConfigToWebgalConfig(textConfig); | ||
| return JSON.stringify(webgalConfig); | ||
| } else { | ||
| return JSON.stringify(defaultWebgalConfig); | ||
| } | ||
| } else { | ||
| const jsonConfigFilePath = this.webgalFs.getPathFromRoot( | ||
| `/public/games/${decodeURI(gameName)}/game/config.json`, | ||
| ); | ||
| return await this.webgalFs.readTextFile(jsonConfigFilePath); | ||
| } | ||
| } |
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.
这个 getGameConfig 方法的实现不够健壮,存在几个问题:
- 返回字符串而不是对象: 当找到
config.txt时,您手动调用JSON.stringify()。然而,NestJS 会自动将控制器方法的返回值序列化为 JSON。这会导致前端收到一个双重编码的 JSON 字符串。您应该直接返回webgalConfig对象。 - 未处理文件不存在的情况: 当
config.txt不存在时,代码会尝试读取config.json。如果config.json也不存在,this.webgalFs.readTextFile会返回字符串'file not exist'。这个字符串会被直接返回给前端,导致前端解析失败。
建议重构此方法,使其在任何情况下都返回一个有效的 IWebgalConfig 对象或默认配置对象,从而增强接口的健壮性。
| const webgalConfig = JSON.parse(gameConfigData.newConfig) as IWebgalConfig; | ||
| const textConfig = convertWebgalConfigToTextConfig( | ||
| webgalConfig, | ||
| ); |
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.
JSON.parse(gameConfigData.newConfig) 可能会因为 newConfig 不是一个有效的 JSON 字符串而抛出异常。这会导致服务器在该请求上崩溃。为了提高服务器的稳定性,您应该将此操作包裹在 try...catch 块中,并对解析失败的情况进行处理,例如返回一个 BadRequestException。
| const webgalConfig = JSON.parse(gameConfigData.newConfig) as IWebgalConfig; | |
| const textConfig = convertWebgalConfigToTextConfig( | |
| webgalConfig, | |
| ); | |
| let webgalConfig: IWebgalConfig; | |
| try { | |
| webgalConfig = JSON.parse(gameConfigData.newConfig); | |
| } catch (e) { | |
| throw new BadRequestException('Invalid config format: not a valid JSON string.'); | |
| } | |
| const textConfig = convertWebgalConfigToTextConfig( | |
| webgalConfig, | |
| ); |
| gameConfig.set(data); | ||
| if (gameConfig.value.gameKey === undefined || gameConfig.value.gameKey === '') { | ||
| // 设置默认识别码 | ||
| const randomCode = (Math.random() * 100000).toString(16).replace(".", "d"); |
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.
当前生成随机码的方式 (Math.random() * 100000).toString(16).replace(".", "d") 有点非常规,并且生成的结果可能不够随机或唯一。例如,它可能会产生像 16462.9122d45d4 这样的字符串。建议使用更标准、更简洁的方式来生成随机字符串,例如 Math.random().toString(36).slice(2)。这会生成一个更干净、同样随机的字符串。这个改动也应该应用到后端的 manage-game.service.ts 文件中,以保持一致性。
| const randomCode = (Math.random() * 100000).toString(16).replace(".", "d"); | |
| const randomCode = Math.random().toString(36).slice(2, 10); |
| <TabItem title={t`游戏名称`}> | ||
| <GameConfigEditor key="gameName" value={getConfigContentAsString('Game_name')} | ||
| onChange={(e: string) => updateGameConfigSimpleByKey("Game_name", e)}/> | ||
| <GameConfigEditor key="gameName" value={gameConfig.value.gameName || defaultWebgalConfig.gameName!} | ||
| onChange={(e: string) => { | ||
| gameConfig.value.gameName = e; | ||
| updateGameConfig(); | ||
| }} | ||
| /> | ||
| </TabItem> | ||
| <TabItem title={t`游戏识别码`}> | ||
| <GameConfigEditor key="gameKey" value={getConfigContentAsString('Game_key')} | ||
| onChange={(e: string) => updateGameConfigSimpleByKey('Game_key', e)}/> | ||
| <GameConfigEditor key="gameKey" value={gameConfig.value.gameKey || defaultWebgalConfig.gameKey!} | ||
| onChange={(e: string) => { | ||
| gameConfig.value.gameKey = e; | ||
| updateGameConfig(); | ||
| }} | ||
| /> | ||
| </TabItem> | ||
| <TabItem title={t`游戏简介`}> | ||
| <GameConfigEditor key="gameDescription" value={getConfigContentAsString('Description')} | ||
| onChange={(e: string) => updateGameConfigSimpleByKey("Description", e)}/> | ||
| <GameConfigEditor key="gameDescription" value={gameConfig.value.description || defaultWebgalConfig.description!} | ||
| onChange={(e: string) => { | ||
| gameConfig.value.description = e; | ||
| updateGameConfig(); | ||
| }} | ||
| /> | ||
| </TabItem> | ||
| <TabItem title={t`游戏包名`}> | ||
| <GameConfigEditor key="packageName" value={getConfigContentAsString('Package_name')} | ||
| onChange={(e: string) => updateGameConfigSimpleByKey('Package_name', e)}/> | ||
| <GameConfigEditor key="packageName" value={gameConfig.value.packageName || defaultWebgalConfig.packageName!} | ||
| onChange={(e: string) => { | ||
| gameConfig.value.packageName = e; | ||
| updateGameConfig(); | ||
| }} | ||
| /> |
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.
在这个组件中,每个配置项的 onChange 回调函数逻辑都非常相似:更新 gameConfig.value 的一个字段,然后调用 updateGameConfig()。这造成了大量的重复代码。为了提高代码的可维护性和简洁性,可以考虑创建一个通用的处理函数来处理这些更新。
例如,您可以创建一个这样的函数:
const handleConfigChange = (key: keyof IWebgalConfig, value: any) => {
gameConfig.value[key] = value;
updateGameConfig();
};然后在 onChange 中调用它:
onChange={(e: string) => handleConfigChange('gameName', e)}
介绍
支持 JSON 格式的游戏配置, 同时兼容旧版本的 txt 格式的游戏配置
更改