-
Notifications
You must be signed in to change notification settings - Fork 340
feat(util): add JAction parallel execution and improve test coverage #607
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
feat(util): add JAction parallel execution and improve test coverage #607
Conversation
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
- Add JActionExecutionContext for per-execution state isolation - Add JActionExecution result struct with per-execution Cancelled state - Add JActionExecutionHandle for per-execution cancellation control - Execute/ExecuteAsync now snapshot tasks enabling safe parallel execution - Switch from ValueTask to UniTask for better Unity integration - Add comprehensive tests for parallel execution, timeouts, and edge cases - Add MessageBox test hooks for pool state inspection - Update jaction skill documentation with new API and patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
4306e9f to
a0eb188
Compare
Unity Test Results✅ EditMode: All tests passed Unity Version: 2022.3.55f1 ✅ All tests passed! The PR is ready for review. View workflow run |
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.
Pull request overview
This PR adds parallel execution support to JAction with per-execution state isolation, switches from ValueTask to UniTask for better Unity integration, and significantly improves test coverage across JEngine.Util and JEngine.UI packages.
Changes:
- Introduces parallel execution mode with
JActionExecutionContextfor per-execution state isolation - Adds
JActionExecutionresult struct andJActionExecutionHandlefor fine-grained execution control - Switches async return types from
ValueTasktoUniTaskfor Unity-native async patterns - Adds comprehensive test coverage for parallel execution, object pooling, edge cases, and timeout scenarios
- Updates jaction skill documentation with new API patterns and parallel execution examples
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| JActionExecutionContext.cs | New internal class holding per-execution state for parallel execution support |
| JActionExecution.cs | New result struct capturing per-execution cancellation state |
| JActionExecutionHandle.cs | New awaitable handle with Cancel() for specific execution control |
| JAction.cs | Refactored to use execution contexts instead of instance-level state |
| JActionAwaitable.cs | Updated to work with execution contexts instead of JAction instances |
| JActionRunner.cs | Updated to process execution contexts instead of JAction instances |
| JEngine.Util.asmdef | Added UniTask dependency reference |
| JActionTests.cs | Added JObjectPool tests and improved edge case coverage |
| JActionRuntimeTests.cs | Added parallel execution, async function, and cancellation tests |
| MessageBox.cs | Added test hooks for pool state inspection (UNITY_INCLUDE_TESTS only) |
| MessageBoxTests.cs | Added comprehensive tests for button visibility, null handling, and concurrency |
| SKILL.md | Updated with parallel execution patterns, return types, and troubleshooting guide |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Runtime/JActionExecutionHandle.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Runtime/MessageBox.cs
Outdated
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.ui/Runtime/MessageBox.cs
Outdated
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
Address CodeQL warnings about potential missed Dispose() calls by wrapping test code in try-finally blocks. Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Fixed
Show fixed
Hide fixed
- Fix Task.CompletedTask -> UniTask.CompletedTask - Fix AsUniTask to delegate to awaiter (prevent double pool return) - Fix Reset to return execution contexts to pool before clearing - Fix MessageBox test hooks to use direct index access (avoid foreach) - Add comment explaining PlayerLoop single-threading in parallel test Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
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.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
Convert try-finally patterns to 'using' declarations for cleaner resource management as suggested by CodeQL static analysis. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
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.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
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.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Runtime/JAction.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
...oject/Packages/com.jasonxudeveloper.jengine.util/Runtime/Internal/JActionExecutionContext.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Editor/JActionTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
UnityProject/Packages/com.jasonxudeveloper.jengine.util/Tests/Runtime/JActionRuntimeTests.cs
Show resolved
Hide resolved
HashSet does not support array indexing. Use LINQ First() method to get the first element from the collection. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
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.
Auto-approved: Copilot review found no issues and Unity Tests passed (or were skipped for non-code changes).
Summary
This PR adds parallel execution capabilities to JAction and improves test coverage for JEngine.Util and JEngine.UI packages.
Key Features
JAction Parallel Execution Mode
Parallel()method: Enable concurrent execution of the same JActionJActionExecutionHandle: Awaitable handle for per-execution control withCancel()methodJActionExecution: Result struct with per-executionCancelledstateExecuteAsync()don't affect running executionsNew Types
JActionExecutionContext: Internal per-execution state for concurrent execution isolationJActionExecution: Lightweight result struct returned when awaiting executionJActionExecutionHandle: Handle for controlling individual executionsTest Coverage Improvements
JEngine.Util
JEngine.UI
Plugin Documentation
Updated
.claude-plugin/skills/jaction/SKILL.mdwith:thisas stateTest plan
🤖 Generated with Claude Code