Custom template font #789
Custom template font #789
Conversation
Summary of ChangesHello @MakinoharaShoko, 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! 此拉取请求的核心是引入了对自定义字体的支持，允许用户通过 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
您好，感谢您对项目的贡献。这次的 PR 引入了对自定义模板字体的支持，这是一个很棒的功能。整体实现结构清晰，通过创建新的工具函数、Redux action 和 reducer 以及新的类型定义，将新功能很好地集成到了现有代码库中。代码的模块化做得很好，例如将模板加载逻辑和字体选项逻辑分别放在了 templateLoader.ts 和 fontOptions.ts 中。
不过，我发现了一些潜在的问题，主要集中在异步操作处理、路径解析和字符串处理上，这些问题可能会导致竞态条件（race condition）、资源加载失败或 CSS 语法错误。具体的修改建议请见下面的评论。
| logger.info(`WebGAL v${__INFO.version}`); | ||
| logger.info('Github: https://github.com/OpenWebGAL/WebGAL '); | ||
| logger.info('Made with ❤ by OpenWebGAL'); | ||
| loadTemplate(); |
There was a problem hiding this comment.
loadTemplate 是一个异步函数，但在这里以“即发即忘”的方式调用，没有等待其完成。这会导致 initializeScript 中的后续代码在模板和字体加载完成前就执行，可能引发竞态条件（race condition）。例如，UI 可能在自定义字体注入前就渲染，导致字体闪烁或初期显示不正确。为了确保初始化按预期顺序完成，initializeScript 函数应该被声明为 async，并且在这里 await loadTemplate()。
| function generateFontFaceRule(font: TemplateFontDescriptor): string | null { | ||
| const fontFamily = font['font-family']; | ||
| if (!fontFamily || !font.url || !font.type) { | ||
| logger.warn('忽略无效的模板字体配置', font); | ||
| return null; | ||
| } | ||
|
|
||
| const src = resolveTemplateAssetPath(font.url); | ||
| const weight = font.weight !== undefined ? `font-weight: ${font.weight};` : ''; | ||
| const style = font.style ? `font-style: ${font.style};` : ''; | ||
| const display = `font-display: ${font.display ?? 'swap'};`; | ||
|
|
||
| return `@font-face { font-family: '${fontFamily}'; src: url('${src}') format('${font.type}'); ${weight} ${style} ${display} }`; | ||
| } |
There was a problem hiding this comment.
generateFontFaceRule 函数在生成 @font-face 规则时，直接用单引号包裹了从 template.json 中获取的 font-family。如果用户在 JSON 文件中提供的字体名称本身已经包含了引号（例如 "'My Font'"），这会导致生成无效的 CSS（例如 font-family: ''My Font''）。建议在处理前先移除字体名称两端可能存在的引号，以增强函数的健壮性。
| function generateFontFaceRule(font: TemplateFontDescriptor): string | null { | |
| const fontFamily = font['font-family']; | |
| if (!fontFamily || !font.url || !font.type) { | |
| logger.warn('忽略无效的模板字体配置', font); | |
| return null; | |
| } | |
| const src = resolveTemplateAssetPath(font.url); | |
| const weight = font.weight !== undefined ? `font-weight: ${font.weight};` : ''; | |
| const style = font.style ? `font-style: ${font.style};` : ''; | |
| const display = `font-display: ${font.display ?? 'swap'};`; | |
| return `@font-face { font-family: '${fontFamily}'; src: url('${src}') format('${font.type}'); ${weight} ${style} ${display} }`; | |
| } | |
| function generateFontFaceRule(font: TemplateFontDescriptor): string | null { | |
| const fontFamily = font['font-family']; | |
| if (!fontFamily || !font.url || !font.type) { | |
| logger.warn('忽略无效的模板字体配置', font); | |
| return null; | |
| } | |
| const sanitizedFontFamily = fontFamily.trim().replace(/^['"]|['"]$/g, ''); | |
| const src = resolveTemplateAssetPath(font.url); | |
| const weight = font.weight !== undefined ? `font-weight: ${font.weight};` : ''; | |
| const style = font.style ? `font-style: ${font.style};` : ''; | |
| const display = `font-display: ${font.display ?? 'swap'};`; | |
| return `@font-face { font-family: '${sanitizedFontFamily}'; src: url('${src}') format('${font.type}'); ${weight} ${style} ${display} }`; | |
| } |
| function resolveTemplateAssetPath(path: string): string { | ||
| if (/^(https?:)?\/\//i.test(path) || path.startsWith('data:')) { | ||
| return path; | ||
| } | ||
| const normalized = path.replace(/^[./]+/, ''); | ||
| return `./game/template/${normalized}`; | ||
| } |
There was a problem hiding this comment.
resolveTemplateAssetPath 函数对相对路径的处理存在问题。当前的实现 path.replace(/^[./]+/,'') 会错误地处理包含 .. 的路径（例如，../fonts/myfont.woff 会被解析到 game/template/fonts/myfont.woff 而不是 game/fonts/myfont.woff），并且也会不正确地处理以 / 开头的根相对路径。这会导致字体等资源无法按预期加载。建议重写此函数，以更准确地处理不同类型的路径。
| function resolveTemplateAssetPath(path: string): string { | |
| if (/^(https?:)?\/\//i.test(path) || path.startsWith('data:')) { | |
| return path; | |
| } | |
| const normalized = path.replace(/^[./]+/, ''); | |
| return `./game/template/${normalized}`; | |
| } | |
| function resolveTemplateAssetPath(path: string): string { | |
| if (/^(https?:)?\/\//i.test(path) || path.startsWith('data:')) { | |
| return path; | |
| } | |
| if (path.startsWith('/')) { | |
| return path; | |
| } | |
| return `./game/template/${path}`; | |
| } |
| export function formatFontFamily(fontFamily: string): string { | ||
| const trimmed = fontFamily.trim(); | ||
| const needsQuote = /\s/.test(trimmed); | ||
| const normalized = needsQuote ? `'${trimmed}'` : trimmed; | ||
| return `${normalized}, serif`; | ||
| } |
There was a problem hiding this comment.
formatFontFamily 函数没有处理 fontFamily 字符串本身已包含引号的情况。如果 template.json 中的字体名是 "'My Font'"，此函数会返回 ''My Font'', serif，这是一个无效的 font-family 值。建议在处理前先移除字符串两端可能存在的引号，以提高代码的健壮性。
| export function formatFontFamily(fontFamily: string): string { | |
| const trimmed = fontFamily.trim(); | |
| const needsQuote = /\s/.test(trimmed); | |
| const normalized = needsQuote ? `'${trimmed}'` : trimmed; | |
| return `${normalized}, serif`; | |
| } | |
| export function formatFontFamily(fontFamily: string): string { | |
| const trimmed = fontFamily.trim().replace(/^['"]|['"]$/g, ''); | |
| const needsQuote = /\s/.test(trimmed); | |
| const normalized = needsQuote ? `'${trimmed}'` : trimmed; | |
| return `${normalized}, serif`; | |
| } |
在模板的描述文件 template.json 中，添加了 font 字段，用于描述用户的自定义字体。这一字体由引擎在初始化阶段通过插入 style 标签的方式被加载到页面中，并在 Options 中注册并成为可选项。
如果有自定义字体，其将会被添加到默认字体之前的位置。并且由于引擎默认选中第一个字体作为对话框和选项等 UI 的字体，所以一般来说自定义字体会被优先选中。