Fix compatibility issue with Reflex 14.3.0#116
Conversation
There was a problem hiding this comment.
Pull request overview
Updates UniState’s Reflex integration to compile and work against Reflex 14.3.0, addressing the interface/API changes introduced in Reflex 14.x (per issues #114 and #115).
Changes:
- Bump Reflex dependency from
11.0.0to14.3.0in Unity package manifest + lockfile. - Replace legacy
AddState*/AddSingleton*binding helpers withRegisterState*/RegisterStateMachine*APIs (keeping the old names as[Obsolete]wrappers). - Update Reflex-based examples and playmode tests to use the new registration APIs and required enums (
Lifetime,Resolution).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates Reflex DI registration documentation to use RegisterState* / RegisterStateMachine* and Lifetime. |
| Packages/packages-lock.json | Locks Reflex to git tag 14.3.0. |
| Packages/manifest.json | Updates Reflex dependency to 14.3.0. |
| Assets/UniStateTests/PlayMode/SubStateTests/SubStateReflexTests.cs | Migrates test bindings to RegisterState* APIs. |
| Assets/UniStateTests/PlayMode/RecoveryTransitionTests/RecoveryReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/PlayMode/HistoryTests/HistorySizeReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/PlayMode/GoToStateTests/GoToReflexTests.cs | Migrates bindings to RegisterState* APIs. |
| Assets/UniStateTests/PlayMode/GoBackToTests/GoBackToReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/PlayMode/GoBackTests/GoBackReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/PlayMode/ExecutionTests/ExecutionReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/PlayMode/BehaviorAttributeTests/BehaviorAttributeReflexTests.cs | Migrates bindings and registers helper via RegisterType(..., Lifetime, Resolution). |
| Assets/UniStateTests/Common/TestFixture/ReflexTestsBase.cs | Migrates base fixture logger registration to RegisterType(..., Lifetime, Resolution). |
| Assets/UniState/Runtime/Integrations/Reflex/ReflexBuildExtensions.cs | Implements new RegisterState* / RegisterStateMachine* helpers and adds [Obsolete] wrappers for old APIs. |
| Assets/Examples/Infrastructure/Reflex/DiceInstaller.cs | Updates example installer registrations to new APIs (including factory registration). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _lifetime = lifetime; | ||
| throw new ArgumentException( | ||
| $"RegisterStateMachine<{stateMachineImplementation.Name}>: Type parameters must differ : " + | ||
| "use RegisterStateMachine() where stateMachineImplementation implements stateMachineContract.\");"); |
There was a problem hiding this comment.
The ArgumentException message for the stateMachineImplementation == stateMachineContract case contains stray escaped characters (\");");) that will show up in the runtime error text and make it confusing. Clean up the string literal so the message reads as intended (and doesn’t include leftover quotes/parentheses/semicolons).
| "use RegisterStateMachine() where stateMachineImplementation implements stateMachineContract.\");"); | |
| "use RegisterStateMachine() where stateMachineImplementation implements stateMachineContract."); |
| { | ||
| throw new ArgumentException( | ||
| $"AddState({state.Name}): Type parameter state must implement IState<TPayload>"); | ||
| $"RegisterState({state.Name}): Type parameter state must implement IState<TPayload>"); |
There was a problem hiding this comment.
ValidateStateBindingInput(Type state) checks IExecutableState, but the thrown message says the type must implement IState<TPayload>. Since non-IState<TPayload> types can still be valid IExecutableState (e.g., sub-state containers), the message is misleading—update it to reference IExecutableState (or whatever the actual required contract is).
| $"RegisterState({state.Name}): Type parameter state must implement IState<TPayload>"); | |
| $"RegisterState({state.Name}): Type parameter state must implement IExecutableState."); |
|
@copilot apply changes based on the comments in this thread |
Resolves #114 #115