4.6#600
Conversation
Allow a game's template to be null across backend, API schema, and frontend. Updates: mark TemplateConfigDto as nullable in Terre DTO, update origine2 API types to TemplateConfigDto | null, add nullable flag to swagger.json, and make TemplateEditorSidebar robust to missing templates by adding helpers to detect and display template names and to filter games using the current template. This prevents runtime errors for games without an associated template and ensures correct selection/display logic.
Make template id optional across API/DTO/swagger and handle built-in default template specially. Frontend: add DEFAULT_TEMPLATE_DIR constant, exclude the built-in default template from template lists, adjust template selection to match by name when id is missing, and use template.dir as Dropdown keys/values. Backend: ManageTemplateService now skips the default dir when scanning, prepends a built-in default TemplateInfo (via getDefaultTemplateInfo), and returns a combined list. UserDataService: add DEFAULT_TEMPLATE_DIR, getDefaultTemplateDir, and resolve helpers so default template files/paths are resolved correctly.
# Conflicts: # package.json # packages/origine2/package.json # packages/origine2/src/locales/ja.po # packages/origine2/src/pages/dashboard/TemplateElement.tsx # packages/terre2/package.json # packages/terre2/src/Modules/manage-template/manage-template.service.ts # packages/terre2/src/main.ts # yarn.lock
fix: getDefaultUserDataRoot on android
fix: 修复 Terre 构建产物缺失 webgal-engine.json
fix: add Intent flag on open browser
![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
This pull request implements a new user data directory system (version 4.6) to unify storage for games, templates, and engines, while adding support for portable mode and legacy data migration. It also updates the UI for settings and game configuration, introduces graphical editing for additional command parameters, and includes various dependency updates. The review feedback identified several technical concerns: fragile argument parsing in the graphical editor, a potential path traversal vulnerability in the file system service, and React anti-patterns regarding ref updates during render, key usage in dynamic lists, and state synchronization in input components.
| const nextArgs = rawArgs | ||
| .split(" -") | ||
| .filter(Boolean) | ||
| .map(arg => arg.trim()) | ||
| .filter(arg => splitArgToken(arg) !== key); | ||
|
|
There was a problem hiding this comment.
This manual argument parsing logic using split(" -") is fragile and can lead to content corruption. If the sentence content contains a space followed by a dash (e.g., in dialogue like say: Wait - what? or in pixi code), it will be incorrectly identified as an argument boundary. Since props.sentence is already a parsed ISentence object, consider manipulating its args property directly and then re-serializing, or use a more sophisticated parsing method that respects the command structure.
| const decodedSrc = decodeURI(src); | ||
| const normalizedSrc = isAbsolute(decodedSrc) | ||
| ? decodedSrc | ||
| : this.getPathFromRoot(decodedSrc); | ||
| const dirPath = join(normalizedSrc, decodeURI(dirName)); |
There was a problem hiding this comment.
Joining normalizedSrc with dirName without validating the resulting path is unsafe. If dirName contains path traversal segments (e.g., ..), the resulting dirPath could point outside the allowed roots. It's recommended to validate the final path using isPathInsideAllowedRoots before calling fs.mkdir.
const dirPath = join(this.normalizeFsPath(src), decodeURI(dirName));
if (!this.isPathInsideAllowedRoots(dirPath)) throw new Error('Path is out of allowed roots');|
|
||
| viewType === 'grid' ? cols.set(gridCols) : cols.set(listCols); | ||
| const columnCount = viewType === 'grid' ? gridCols : listCols; | ||
| colsRef.current = columnCount; |
There was a problem hiding this comment.
Updating a ref during the render phase is a side effect and should be avoided in React. This can lead to unpredictable behavior, especially with concurrent rendering or when React optimizes renders. It's better to perform such updates in a useEffect or useLayoutEffect hook that responds to changes in width or viewType.
| const chooseList = chooseItems.value.map((item, i) => { | ||
| return <div style={{ display: "flex", width: '100%', alignItems: "center", padding: '0 0 4px 0' }} key={i}> | ||
| const target = item[1] ?? ""; | ||
| return <div className={styles.chooseBranchItem} key={i}> |
There was a problem hiding this comment.
Using the array index i as a key in a dynamic list where items have local state (like ChooseTargetDialog) can lead to UI synchronization bugs when items are added or removed. React may reuse component instances for different data items. It's recommended to use a unique identifier for each branch.
| const title = useValue(argText(titleFromArgs, true)); | ||
| const buttonTextFromArgs = getArgByKey(props.sentence, "buttonText"); | ||
| const buttonText = useValue(((buttonTextFromArgs === 0 ? '' : buttonTextFromArgs) ?? '').toString()); | ||
| const buttonText = useValue(argText(buttonTextFromArgs, true)); | ||
| const defaultValueFromArgs = getArgByKey(props.sentence, "defaultValue"); | ||
| const defaultValue = useValue(argText(defaultValueFromArgs)); | ||
| const rule = useValue(argText(getArgByKey(props.sentence, "rule"), true)); | ||
| const ruleFlag = useValue(argText(getArgByKey(props.sentence, "ruleFlag"), true)); | ||
| const ruleText = useValue(argText(getArgByKey(props.sentence, "ruleText"), true)); | ||
| const ruleButtonText = useValue(argText(getArgByKey(props.sentence, "ruleButtonText"), true)); |
There was a problem hiding this comment.
These local states are initialized from props but are not synchronized when the props change. If the sentence data is updated externally (e.g., via an Undo action or a global search/replace), this component will continue to display stale local state until it is unmounted and remounted. You should add a useEffect hook to synchronize these values whenever props.sentence changes, similar to the pattern used in AppSettingsDialog.tsx.
3 participants
No description provided.