Skip to content

fix(manage_gameobject): default prefab_folder to Assets/Prefabs - #1496

Open
mmporong wants to merge 1 commit into
CoplayDev:betafrom
mmporong:fix/prefab-folder-default
Open

mmporong wants to merge 1 commit into
CoplayDev:betafrom
mmporong:fix/prefab-folder-default

Conversation

@mmporong

@mmporong mmporong commented Oct 26, 2026 •

Copy link
Copy Markdown

Description

manage_gameobject(action="create", name="Enemy", save_as_prefab=True) without prefab_path sends prefabPath = "None/Enemy.prefab" to Unity.

The default path is built from prefab_folder, and the comment says that parameter "has a default". It did originally (prefab_folder: str = "Assets/Prefabs" in cb603b7), but since the parameter became Annotated[str, ...] | None = None its default is None, so the f-string produces "None/<name>.prefab". Reading GameObjectCreate.cs, that path goes to Directory.CreateDirectory and PrefabUtility.SaveAsPrefabAssetAndConnect outside Assets/ (not run in Unity).

This PR falls back to "Assets/Prefabs" when prefab_folder is not given. An explicit prefab_folder or prefab_path behaves as before.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Test update

Changes Made

  • Server/src/services/tools/manage_gameobject.py: use prefab_folder or "Assets/Prefabs" when constructing the default prefab path, and update the comment.
  • Server/tests/test_manage_gameobject.py: add TestManageGameObjectPrefabPath (default folder, explicit prefab_folder, explicit prefab_path), using the existing mock_unity fixture.

Compatibility / Package Source

  • Unity version(s) tested: none (Python server change only; not run in Unity)
  • Package source used (#beta, #main, tag, branch, or file:): not applicable
  • Resolved commit hash from Packages/packages-lock.json (if using a Git package URL): not applicable

Testing/Screenshots/Recordings

  • Python tests (cd Server && uv run pytest tests/ -v)
  • Unity EditMode tests
  • Unity PlayMode tests
  • Package import/compile check
  • Not applicable (explain why in Additional Notes)

Before the fix, the new default-folder test fails with assert 'None/Enemy.prefab' == 'Assets/Prefabs/Enemy.prefab'. After the fix, uv run --extra dev pytest tests -q --ignore=tests/e2e passes: 1579 passed, 18 skipped.

Documentation Updates

  • I have added/removed/modified tools or resources
  • If yes, I have updated all documentation files using:
    • The LLM prompt at tools/UPDATE_DOCS_PROMPT.md (recommended)
    • Manual review of the generated changes

No tool or parameter was added or renamed; only the default used for an omitted prefab_folder changes.

Related Issues

None.

AI assistance: this change was prepared with Claude (claude-opus-5-5), which found the bug, wrote the fix and tests, ran the test suite, and drafted this description.

Summary by CodeRabbit

  • Bug Fixes
    • Prefabs created without a specified path or folder are now saved under Assets/Prefabs using the GameObject’s name. Specifying a folder or an explicit path continues to use that location.

With save_as_prefab and no prefab_path, the default prefab path is built
from prefab_folder, which the comment says has a default. Since the
parameter moved to an Annotated[...] | None = None signature its default
is None, so the path became "None/<name>.prefab" and Unity was asked to
save the prefab outside Assets.

Fall back to "Assets/Prefabs", the original default, when prefab_folder
is not given, and add tests for the default, an explicit folder and an
explicit prefab_path.

Assisted-by: Claude:claude-opus-5-5
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c98deb4f-58d8-4f67-b9cb-30c04fee9d29
Commits

Reviewing files that changed from the base of the PR and between ed4e205 and a569593.

Files selected for processing: 2
  • Server/src/services/tools/manage_gameobject.py
  • Server/tests/test_manage_gameobject.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Prefab creation now uses Assets/Prefabs when neither an explicit prefab path nor a folder is supplied. Tests cover the default path, a specified folder, and an explicit path.

Changes: Prefab path handling

Layer / File(s) Summary
Resolve prefab creation paths
Server/src/services/tools/manage_gameobject.py, Server/tests/test_manage_gameobject.py
When no explicit path or folder is supplied, prefab creation uses Assets/Prefabs/<name>.prefab and omits prefabFolder. Tests also check custom-folder and explicit-path behavior.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: msanatan

Merge Risk: Minimal · up to a5695

The scoped prefab path change is supported by focused coverage and presents minimal merge risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the bug fix and the restored default value for prefab_folder.
Description check Passed The description follows the repository template, explains the defect and fix, lists affected files, records compatibility details, identifies the test command and results, and explains why documentati…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description follows the repository template, explains the defect and fix, lists affected files, records compatibility details, identifies the test command and results, and explains why documentation and Unity tests were not required.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant