Skip to content

feat: support project font persistence - #3362

Merged
nighca merged 4 commits into
goplus:devfrom
nighca:issue-3342-project-fonts
Aug 7, 2026
Merged

nighca merged 4 commits into
goplus:devfrom
nighca:issue-3342-project-fonts

Conversation

@nighca

@nighca nighca commented Jul 21, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #3343

Adds Builder support for SPX project fonts:

  • persist font families under assets/fonts/<family>/ and the fontPreferences array in assets/index.json;
  • preserve fonts and preferences through project load/save/export;
  • initialize new projects from deployment-specific default preferences, including the domestic basic-Chinese preset;
  • assemble the default project from lazily loaded source assets through the project model APIs instead of an .xbp template;
  • remove Builder-bundled Scratch fonts while retaining the generic SVG font-injection mechanism.

Editor previews intentionally do not consume project fonts or preferences in this PR. That integration is tracked separately in #3366 and will be implemented by #3348 after its large-font SVG rendering performance issue is resolved.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for project-specific font preferences and generic font embedding in SVGs, replacing the previous Scratch-specific implementation. It adds a new FontFamily model, updates SpxProject to manage and export font preferences, and implements a utility to inject referenced font files as base64 data URIs into SVGs. The code reviewer provided valuable feedback to improve performance and robustness: they suggested using the asynchronous FileReader.readAsDataURL API instead of a synchronous manual chunking loop to avoid blocking the main thread when encoding large font files, and recommended filtering out empty strings when parsing font preferences and extracting used font families to handle malformed inputs gracefully.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread spx-gui/src/utils/svg-font.ts
Comment thread spx-gui/src/utils/svg-font.ts
Comment thread spx-gui/src/models/spx/project.ts Outdated

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review Summary

This PR cleanly adds a project-font persistence layer (FontFamily model, project.fonts / fontPreferences, config round-trip, legacy-project migration) with good persistence-side test coverage, and refactors the Scratch-specific SVG font injection into a generic injectFontsToSvgText(svgText, fontPreferences, fontFiles).

The main concern is that the new mechanism is not actually wired into the runtime rendering path, which makes the feature inert at render time and regresses the previous Scratch-font embedding. A few smaller robustness/perf items follow. Security review found no exploitable issues (CSS font-family values are escaped via JSON.stringify, style is injected through XMLSerializer, and faces[].path is only used for in-memory map lookups, not filesystem access).

Inline comments cover the concrete diff-line findings. Additional items that don't map to a single changed line:

  • maintainability — duplicated quote-stripping: stripQuotes in spx-gui/src/utils/svg-font.ts:9 and stripFontPreferenceQuotes in spx-gui/src/models/spx/project.ts:40 are byte-for-byte identical. Extract a single shared helper so preference parsing and SVG rule generation can't diverge on family-name semantics.
  • maintainability — migration condition coupling: spx-gui/src/models/spx/project.ts:469 triggers legacy migration only when fontPreferences == null && fonts.length === 0. The null check alone is the real signal of a pre-feature project; the extra fonts.length === 0 clause is subtle and undocumented. Consider keying migration on fontPreferences == null only, or add a comment explaining why both are required.
  • stale comment (outside this diff): spx-gui/src/utils/file.ts:125 still says useRenderableImageUrl "applies rendering-specific handling such as Scratch font injection for SVG files." Since scratch-svg-font.ts was removed, this should read "font injection for SVG files."
  • licensing hygiene: the deleted scratch/ fonts vendored their license text (OFL.txt / LICENSE.txt), but the new basic-chinese/README.md only links to the source repo. If policy is to vendor the OFL text alongside binary font assets, consider adding it.
  • test gap: spx-gui/src/utils/img-rendering.test.ts fully mocks injectFontsToSvgText, so no test asserts what arguments the real render path passes — which is why the wiring gap below is invisible to CI. Once wiring is added, assert that getRenderableImageUrl forwards the project's preferences and font files.

Comment thread spx-gui/src/utils/img-rendering.ts
Comment thread spx-gui/src/utils/svg-font.ts
Comment thread spx-gui/src/models/spx/project.ts Outdated
Comment thread spx-gui/src/models/spx/project.ts
@nighca
nighca force-pushed the issue-3342-project-fonts branch from 1d9307a to 1a0b50b Compare July 22, 2026 05:53
@nighca

nighca commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Review-summary follow-up:

  • The empty-family cases and the stale useFileUrl comment were fixed in 1a0b50b.
  • I kept the project-model parser and generic SVG parser local: they serve separate layers and remain small.
  • I kept the migration predicate: only the absence of both preferences and collection identifies the old no-font configuration; a project already carrying fonts is not rewritten.
  • The bundled Noto CJK asset is temporary and will be replaced, so we are not adding a separate license text file now.
  • Project-font rendering and its end-to-end tests remain in feat: render project fonts in editor #3348, which owns editor integration and the ongoing rendering-performance work.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

把原有的 default-project.xbp 拆了(这样方便根据不同的 env 构造不同的 project 内容),所以多了个一个新的 ‎spx-gui/src/components/project/default-project/ 目录

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

svg-font.ts 是原来的 scratch-svg-font.ts 干掉 scratch 的特有逻辑后得到的,不过它的逻辑事实上没生效,只是先保留这份逻辑,后续再更新并启用

@CORCTON

CORCTON commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

It appears that it is possible to create font configurations that Builder accepts but SPX rejects. For example:

  • A font family named DEFAULT;
  • Two font families named Foo and foo;
  • A preference that references a font family that does not exist in the project;
  • A preference that includes both Foo and foo.

Currently, Builder only rejects empty font family names, exact matches of the lowercase name default, and cases where names are exactly duplicated, whereas SPX validates reserved names, duplicate names, and the validity of preferences after performing ASCII case conversion.

For these cases, is runtime rejection the expected behavior, or does Builder apply an equivalent validation mechanism elsewhere?

@nighca

nighca commented Aug 3, 2026 •

Copy link
Copy Markdown
Collaborator Author

Currently, Builder only rejects empty font family names, exact matches of the lowercase name default, and cases where names are exactly duplicated, whereas SPX validates reserved names, duplicate names, and the validity of preferences after performing ASCII case conversion.

For these cases, is runtime rejection the expected behavior, or does Builder apply an equivalent validation mechanism elsewhere?

@CORCTON It's intended. Builder-side validation aims to enhance user experience by catching common errors and providing immediate feedback. While not replicating spx validation exactly, we focus on frequently encountered or easily verifiable mistakes to avoid unnecessary complexity.

@nighca
nighca force-pushed the issue-3342-project-fonts branch from 435b65f to 4089414 Compare August 7, 2026 07:54
@nighca
nighca merged commit e6a9eb6 into goplus:dev Aug 7, 2026
5 checks passed
aofei added a commit that referenced this pull request Aug 14, 2026
* Sync `main` to `dev` (#3398)

* chore(deps): bump spx to 3.1.0 (#3397)

* chore(deps): bump spx to 3.1.0

* fix(deps): add spx 3.1.0 module checksums

* fix(deps): record spx transitive module requirement

* fix(deps): tidy spx 3.1.0 module graph

* feat: support project font persistence (#3362)

* feat: support project font persistence

* chore: use SPX preset font assets

* fix: address project font review comments

* docs: link deferred project font rendering issue

* fix(ai): handle interaction termination consistently (#3399)

Schedule history management from the interaction cleanup path so completed
sequences are archived even when they return early. Run archive work in an
owner-scoped coroutine so it survives caller completion without outliving
the game.

Report an error when an interaction exhausts its turn limit instead of
silently treating an unfinished sequence as complete.

Add regression coverage for archive lifecycle behavior and turn-limit
failure reporting.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* feat(account): add Firefox browser hijacking (#3404)

Support Firefox WebDriver BiDi alongside Chrome DevTools Protocol for
local Account development. Preserve redirects for sign-in and identity
provider callbacks across both browsers.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* fix(api): raise the course prompt limit to 12000 (#3408)

A course prompt carries the scaffold code the learner starts from, the
reference answer, and the rules that decide when the course is complete, so
exercise and multi-sprite courses run several times longer than a plain
lesson. The longest published course prompt is 8763 characters, past the 4000
the contract has allowed since #3279.

Mirrors goplus/builder-backend#329, which relaxes the server-side check.

Co-authored-by: Ethanlita <a1198537235@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* docs(product): add product design for the new tutorial course system (#3402)

* docs(product): add product design for the new tutorial course system

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: split tutorial v2 into product design and develop notes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(product): add English version of tutorial v2 design

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix(spx-gui): timeout pre-run static checks (#3410)

* fix(spx-gui): timeout pre-run static checks

* fix(spx-gui): capture static check timeouts

* fix(spx-gui): bound pre-run static checks

* refactor(spx-gui): name pre-run static checks

* refactor(spx-gui): simplify static check timeout handling

* docs(spx-gui): explain monitor check cancellation

* fix(spx-gui): include capture context in Sentry

* fix(spx-gui): scope static check timeouts to LS calls

* refactor(spx-gui): reuse static check timeout helper

* refactor(spx-gui): reuse timeout helper for thumbnails

* docs(spx-gui): explain Code Editor timeout

* fix(spx-gui): refine Code Editor timeout handling

* refactor(spx-gui): merge timeout signals in helper

* fix(spx-gui): race external timeout signals

* docs(spx-gui): explain pre-run check errors

* docs(spx-gui): clarify pre-run error handling

* fix(account): preserve client IP through trusted proxies (#3389)

Trust only the jfcs-k8s-qa1 ingress controller addresses when restoring
`X-Real-IP` before proxying Account API requests. This preserves the
client address across the hosted sign-in facade for backend IP-based
limits.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* fix(editor): avoid quadratic input slot filtering (#3415)

Upgrade xgolsw to collect large input-slot sets efficiently and rely on
its pairwise non-overlap guarantee. Remove the redundant editor-side
containment pass so projects with many editable literals stay responsive.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

---------

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
Co-authored-by: Aofei Sheng <aofei@aofeisheng.com>
Co-authored-by: Ethanlita <65023676+Ethanlita@users.noreply.github.com>
Co-authored-by: Ethanlita <a1198537235@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
aofei added a commit that referenced this pull request Aug 14, 2026
* Sync `main` to `dev` (#3398)

* chore(deps): bump spx to 3.1.0 (#3397)

* chore(deps): bump spx to 3.1.0

* fix(deps): add spx 3.1.0 module checksums

* fix(deps): record spx transitive module requirement

* fix(deps): tidy spx 3.1.0 module graph

* feat: support project font persistence (#3362)

* feat: support project font persistence

* chore: use SPX preset font assets

* fix: address project font review comments

* docs: link deferred project font rendering issue

* fix(ai): handle interaction termination consistently (#3399)

Schedule history management from the interaction cleanup path so completed
sequences are archived even when they return early. Run archive work in an
owner-scoped coroutine so it survives caller completion without outliving
the game.

Report an error when an interaction exhausts its turn limit instead of
silently treating an unfinished sequence as complete.

Add regression coverage for archive lifecycle behavior and turn-limit
failure reporting.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* feat(account): add Firefox browser hijacking (#3404)

Support Firefox WebDriver BiDi alongside Chrome DevTools Protocol for
local Account development. Preserve redirects for sign-in and identity
provider callbacks across both browsers.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* fix(api): raise the course prompt limit to 12000 (#3408)

A course prompt carries the scaffold code the learner starts from, the
reference answer, and the rules that decide when the course is complete, so
exercise and multi-sprite courses run several times longer than a plain
lesson. The longest published course prompt is 8763 characters, past the 4000
the contract has allowed since #3279.

Mirrors goplus/builder-backend#329, which relaxes the server-side check.

Co-authored-by: Ethanlita <a1198537235@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* docs(product): add product design for the new tutorial course system (#3402)

* docs(product): add product design for the new tutorial course system

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: split tutorial v2 into product design and develop notes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(product): add English version of tutorial v2 design

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix(spx-gui): timeout pre-run static checks (#3410)

* fix(spx-gui): timeout pre-run static checks

* fix(spx-gui): capture static check timeouts

* fix(spx-gui): bound pre-run static checks

* refactor(spx-gui): name pre-run static checks

* refactor(spx-gui): simplify static check timeout handling

* docs(spx-gui): explain monitor check cancellation

* fix(spx-gui): include capture context in Sentry

* fix(spx-gui): scope static check timeouts to LS calls

* refactor(spx-gui): reuse static check timeout helper

* refactor(spx-gui): reuse timeout helper for thumbnails

* docs(spx-gui): explain Code Editor timeout

* fix(spx-gui): refine Code Editor timeout handling

* refactor(spx-gui): merge timeout signals in helper

* fix(spx-gui): race external timeout signals

* docs(spx-gui): explain pre-run check errors

* docs(spx-gui): clarify pre-run error handling

* fix(account): preserve client IP through trusted proxies (#3389)

Trust only the jfcs-k8s-qa1 ingress controller addresses when restoring
`X-Real-IP` before proxying Account API requests. This preserves the
client address across the hosted sign-in facade for backend IP-based
limits.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

* fix(editor): avoid quadratic input slot filtering (#3415)

Upgrade xgolsw to collect large input-slot sets efficiently and rely on
its pairwise non-overlap guarantee. Remove the redundant editor-side
containment pass so projects with many editable literals stay responsive.

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>

---------

Signed-off-by: Aofei Sheng <aofei@aofeisheng.com>
Co-authored-by: Hanxing Yang <nighca@live.cn>
Co-authored-by: Ethanlita <65023676+Ethanlita@users.noreply.github.com>
Co-authored-by: Ethanlita <a1198537235@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – builder — 40894149 Deployed Aug 7, 2026 by vercel[bot]
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.

Implement project-font persistence and defaults in XBuilder

2 participants