-
Notifications
You must be signed in to change notification settings - Fork 487
Update .Bat file and Bug fix on ManageScript #355
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
Conversation
* Update the .Bat file to include runtime folder * Fix the inconsistent EditorPrefs variable so the GUI change on Script Validation could cause real change.
WalkthroughUpdates adjust how script validation level is stored in the Unity Editor prefs, change default paths in deployment scripts, and split restore logic to treat Unity Bridge Editor and Runtime as separate restoration targets with distinct prompts and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant restore-dev.bat as RestoreScript
participant FS
Note over RestoreScript: Start restore flow
User->>RestoreScript: Select restore
RestoreScript->>FS: Check for Unity Bridge Editor backup
alt Editor backup exists
RestoreScript->>User: Prompt "Restore Unity Bridge Editor?"
User-->>RestoreScript: Confirm/Cancel
RestoreScript->>FS: Restore Editor backup
RestoreScript-->>User: Show Editor success/error
else Editor missing
RestoreScript-->>User: Show Editor missing warning
end
RestoreScript->>FS: Check for Unity Bridge Runtime backup
alt Runtime backup exists
RestoreScript->>User: Prompt "Restore Unity Bridge Runtime?"
User-->>RestoreScript: Confirm/Cancel
RestoreScript->>FS: Restore Runtime backup
RestoreScript-->>User: Show Runtime success/error
else Runtime missing
RestoreScript-->>User: Skip Runtime (no backup)
end
RestoreScript->>FS: Check/Restore Python Server backup (unchanged)
RestoreScript-->>User: Final summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy-dev.bat (1)
94-124: Critical: Runtime folder not backed up or deployed despite PR objectivesThe PR objectives state "Update the .Bat file to include the runtime folder," but deploy-dev.bat only backs up and deploys the Editor folder (lines 96-104 and 119-124). The Runtime folder is neither backed up nor deployed.
This creates a critical problem because restore-dev.bat (lines 135-146) attempts to restore Runtime files from backups that were never created.
Add Runtime folder backup after the Editor backup section:
) ) +if exist "%PACKAGE_CACHE_PATH%\Runtime" ( + echo Backing up Unity Bridge Runtime files... + xcopy "%PACKAGE_CACHE_PATH%\Runtime" "%BACKUP_SUBDIR%\UnityBridge\Runtime\" /E /I /Y > nul + if !errorlevel! neq 0 ( + echo Error: Failed to backup Unity Bridge Runtime files + pause + exit /b 1 + ) +) + :: Deploy Unity BridgeAdd Runtime folder deployment after the Editor deployment section:
exit /b 1 ) +:: Deploy Unity Bridge Runtime +echo Deploying Unity Bridge Runtime code... +xcopy "%BRIDGE_SOURCE%\Runtime\*" "%PACKAGE_CACHE_PATH%\Runtime\" /E /Y > nul +if !errorlevel! neq 0 ( + echo Error: Failed to deploy Unity Bridge Runtime code + pause + exit /b 1 +) + :: Deploy Python Server
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
MCPForUnity/Editor/Tools/ManageScript.cs(1 hunks)deploy-dev.bat(1 hunks)restore-dev.bat(2 hunks)
🔇 Additional comments (1)
restore-dev.bat (1)
105-106: Good: Runtime restoration structure is well-implementedThe addition of separate Editor and Runtime restoration paths (lines 105-106, 135-146) is well-structured and follows the same error handling pattern as the existing Editor restoration. The code appropriately handles cases where Runtime backups don't exist.
However, this functionality depends on the critical fix needed in deploy-dev.bat to actually create Runtime backups. Until deploy-dev.bat is updated to backup the Runtime folder, the Runtime restoration will always skip with a warning.
Note: This comment assumes the deploy-dev.bat fixes flagged in my earlier review comments will be applied.
Also applies to: 135-146
String to Int for consistency
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
MCPForUnity/Editor/Tools/ManageScript.cs(1 hunks)restore-dev.bat(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- MCPForUnity/Editor/Tools/ManageScript.cs
🔇 Additional comments (3)
restore-dev.bat (3)
105-106: Clear separation of Editor and Runtime targets.The updated messages effectively communicate that Editor and Runtime components are restored to separate directories, improving transparency for users running the restore operation.
122-133: Improved messaging for Editor restoration.The refined messaging clearly identifies the Editor component being restored, maintaining the same robust error handling while improving clarity for troubleshooting.
14-14: Path consistency verified—no changes required.The
DEFAULT_SERVER_PATHinrestore-dev.batline 14 matches the corresponding path indeploy-dev.batline 14. Both scripts use the path consistently throughout their workflows.
| if exist "%SELECTED_BACKUP%\UnityBridge\Runtime" ( | ||
| echo Restoring Unity Bridge Runtime files... | ||
| rd /s /q "%PACKAGE_CACHE_PATH%\Runtime" 2>nul | ||
| xcopy "%SELECTED_BACKUP%\UnityBridge\Runtime\*" "%PACKAGE_CACHE_PATH%\Runtime\" /E /I /Y > nul | ||
| if !errorlevel! neq 0 ( | ||
| echo Error: Failed to restore Unity Bridge Runtime files | ||
| pause | ||
| exit /b 1 | ||
| ) | ||
| ) else ( | ||
| echo Warning: No Unity Bridge Runtime backup found, skipping... | ||
| ) |
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.
🧩 Analysis chain
Runtime restoration block successfully implements PR objective.
The new Runtime restoration logic correctly mirrors the Editor block with appropriate error handling and messaging. This addresses the PR objective to include the runtime folder in the restore workflow.
Verify that the backup script (backup-dev.bat) has been updated to create the UnityBridge\Runtime directory structure, ensuring backups contain the necessary Runtime files for successful restoration.
🏁 Script executed:
#!/bin/bash
# Description: Verify backup script creates UnityBridge\Runtime directory structure
# Search for Runtime backup logic in backup-dev.bat
rg -n "Runtime|UnityBridge" -g "backup-dev.bat" -C3Length of output: 148
🏁 Script executed:
#!/bin/bash
# Description: Locate and examine backup-dev.bat for Runtime directory handling
# First, find the backup-dev.bat file
echo "=== Locating backup-dev.bat ==="
fd -n "backup-dev.bat"
# Then search for Runtime-related content
echo ""
echo "=== Searching for Runtime in backup script ==="
rg -n "Runtime" -C3Length of output: 85453
🏁 Script executed:
#!/bin/bash
# Description: Locate backup-dev.bat and verify Runtime backup logic
# Find backup-dev.bat using correct fd syntax
echo "=== Finding backup-dev.bat ==="
fd "backup-dev.bat"
# Once located, examine its contents for UnityBridge\Runtime
echo ""
echo "=== Examining backup-dev.bat for Runtime backup logic ==="
find . -name "backup-dev.bat" -type f -exec cat {} \;Length of output: 89
🏁 Script executed:
#!/bin/bash
# Description: Search for backup-related scripts
echo "=== All .bat files in repository ==="
find . -name "*.bat" -type f
echo ""
echo "=== All .sh and .ps1 files ==="
find . -name "*.sh" -o -name "*.ps1" | head -20
echo ""
echo "=== Search for 'backup' in script filenames ==="
find . -type f \( -name "*backup*" \)
echo ""
echo "=== Check if restore-dev.bat references backup-dev.bat ==="
grep -n "backup-dev" restore-dev.bat 2>/dev/null || echo "No reference found"Length of output: 266
Backup script does not exist — restore functionality cannot work.
The restore block at lines 135-146 assumes backups exist at %SELECTED_BACKUP%\UnityBridge\Runtime, but there is no backup-dev.bat in the repository and no backup functionality at all. The Runtime restore code will always skip with the warning message, rendering it non-functional.
Implement backup functionality (at minimum, a backup-dev.bat script that creates backups of %PACKAGE_CACHE_PATH%\Runtime to the backup location) to enable the restore workflow.
🤖 Prompt for AI Agents
In restore-dev.bat around lines 135 to 146, the restore assumes a backup exists
at %SELECTED_BACKUP%\UnityBridge\Runtime but no backup script or backups are
provided; add a new backup-dev.bat that creates and verifies backups so restore
can work: implement a batch that validates %PACKAGE_CACHE_PATH%\Runtime exists,
creates the %BACKUP_ROOT% (or %SELECTED_BACKUP%) folder if missing, copies the
Runtime folder into a timestamped subfolder (or into the chosen backup path)
using robocopy/xcopy with proper flags, returns a non-zero exit code and prints
an error on failure, and documents expected environment variables; ensure the
backup script sets/exports the same variables used by restore-dev.bat and exits
successfully when backup completes so the restore block can find and restore the
files.
Summary by CodeRabbit