Release 0.11.0 #20
Release 0.11.0 #20
Conversation
- Added two new static event calls to UiService that are triggered when the app changes resolution and another trigger when changes screen orientation - The new *UiServiceMonoComponent* was added to the project for internal purposes, in order to support Unity's loop or generic *GameObjects* dependency code (e.g. the screen resolution trigger change) - Added new *AdjustScreenSizeFitter* to extend the ui behaviour of Unity's *ContentSizeFitter* in order to allow the *LayoutElement* to fit between the *LayoutElement.minSize* & *LayoutElement.flexibleSize*, values defined in the Unity's inspector **Changed**: - Renamed *UiPresenterData<T>* to *UiPresenter<T>* to make it less verbose
📝 WalkthroughWalkthroughThis pull request introduces version 0.11.0 of the UI service library, focusing on enhancing screen resolution and orientation handling. The changes include renaming the Changes
Sequence DiagramsequenceDiagram
participant UI as UiService
participant Mono as UiServiceMonoComponent
participant Screen as Screen/Device
Screen->>Mono: Resolution/Orientation Change
Mono->>UI: Trigger OnResolutionChanged
Mono->>UI: Trigger OnOrientationChanged
UI->>UI: Update UI Components
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Runtime/AdjustScreenSizeFitter.cs (1)
59-70: Avoid repeated size checks every frame if unnecessary.
UsingLateUpdateto check for resolution changes is acceptable; however, for performance-sensitive applications, you could provide a way to disable or reduce checks if the resolution rarely changes (e.g., on non-mobile platforms). This optimization might eliminate superfluous per-frame calls.Runtime/UiServiceMonoComponent.cs (1)
21-28: Consider throttling or debouncing resolution change events.
When the resolution fluctuates rapidly (e.g., device transitions or user resizing windows), it may invokeUiService.OnResolutionChangedrepeatedly. If the app performs complex operations in this callback, consider implementing throttling or debouncing logic to minimize excessive calls.Runtime/UiService.cs (1)
255-261: Write more descriptive error logs.
When the cast toUiPresenter<TData>fails, the log incorrectly states "Implement it to allow it to open with initial defined data". Consider specifying the actual type mismatch to help debugging.- Debug.LogError($"The UiPresenter {type} is not a UiPresenter<TData> type you. " + - $"Implement it to allow it to open with initial defined data"); + Debug.LogError($"Failed to cast {type} to UiPresenter<{typeof(TData)}>." + + "Ensure that the UI prefab implements the correct generic presenter type.");CHANGELOG.md (1)
9-13: Improve clarity and reduce wordiness in feature descriptions.The features are well-documented, but the text can be more concise and grammatically correct.
Apply these improvements:
**New**: -Added two new static event calls to UiService that are triggered when the app changes resolution and another trigger when changes screen orientation -The new *UiServiceMonoComponent* was added to the project for internal purposes, in order to support Unity's loop or generic *GameObjects* dependency code (e.g. the screen resolution trigger change) -Added new *AdjustScreenSizeFitter* to extend the ui behaviour of Unity's *ContentSizeFitter* in order to allow the *LayoutElement* to fit between the *LayoutElement.minSize* & *LayoutElement.flexibleSize*, values defined in the Unity's inspector +- Added two new static events to UiService: one for app resolution changes and another for screen orientation changes +- Added *UiServiceMonoComponent* to support Unity's loop and *GameObject* dependencies for screen resolution changes +- Added *AdjustScreenSizeFitter* to extend Unity's *ContentSizeFitter*, allowing *LayoutElement* to fit between *LayoutElement.minSize* and *LayoutElement.flexibleSize* defined in Unity's inspector🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: Possible agreement error. The noun ‘event’ seems to be countable.
Context: ...-05New:
- Added two new static event calls to UiService that are triggered w...
(CD_NN)
[style] ~11-~11: Consider a shorter alternative to avoid wordiness.
Context: ...d to the project for internal purposes, in order to support Unity's loop or generic *GameOb...(IN_ORDER_TO_PREMIUM)
[style] ~12-~12: Consider a shorter alternative to avoid wordiness.
Context: ...ehaviour of Unity's ContentSizeFitter in order to allow the LayoutElement to fit betwee...(IN_ORDER_TO_PREMIUM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)Runtime/AdjustScreenSizeFitter.cs(1 hunks)Runtime/AdjustScreenSizeFitter.cs.meta(1 hunks)Runtime/UiPresenter.cs(1 hunks)Runtime/UiService.cs(4 hunks)Runtime/UiServiceMonoComponent.cs(1 hunks)Runtime/UiServiceMonoComponent.cs.meta(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- package.json
- Runtime/AdjustScreenSizeFitter.cs.meta
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: Possible agreement error. The noun ‘event’ seems to be countable.
Context: ...-05
New:
- Added two new static event calls to UiService that are triggered w...
(CD_NN)
[style] ~11-~11: Consider a shorter alternative to avoid wordiness.
Context: ...d to the project for internal purposes, in order to support Unity's loop or generic *GameOb...
(IN_ORDER_TO_PREMIUM)
[style] ~12-~12: Consider a shorter alternative to avoid wordiness.
Context: ...ehaviour of Unity's ContentSizeFitter in order to allow the LayoutElement to fit betwee...
(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (9)
Runtime/AdjustScreenSizeFitter.cs (1)
39-45: Consider handling missing Canvas scenarios.
IfGetComponentInParent<Canvas>()returnsnull, callingGetComponent<RectTransform>()may lead to aNullReferenceException. You may want to add a null check or gracefully handle the case where a Canvas is not found, to prevent runtime errors in unconventional setups or test scripts.Would you like me to generate a shell script to identify all references to
GetComponentInParent<Canvas>()and check if they handle null scenarios?Runtime/UiServiceMonoComponent.cs (1)
30-45: Handle orientation transitions beyond standard states.
While ignoringUnknown,FaceUp, andFaceDownis fine, certain devices might briefly report these orientations while rotating, causing potential missed triggers. Evaluating how quickly you check transitions may help avoid skipping relevant orientation events on rapidly rotating devices.Runtime/UiPresenter.cs (1)
89-89: Renaming toUiPresenter<T>is more concise.
This rename effectively reduces verbosity. Ensure any existing references to the old name, especially in documentation or third-party integrations, are updated to prevent confusion.Runtime/UiService.cs (3)
15-16: Events for resolution/orientation changes look good.
These static readonlyUnityEventfields add flexibility for listening to UI changes across the system. Just be mindful of event subscribers to avoid memory leaks if you add and forget to remove listeners.
66-66: ConsiderDontDestroyOnLoadusage.
AttachingUiServiceMonoComponentand callingDontDestroyOnLoadensures the UI persists between scene loads. Confirm this is desired in all contexts; if the UI is scene-specific, you may not want it to persist.
71-71: Appropriate async call usage.
UsingLoadUiAsync(_loadingSpinnerType).Forget()clarifies that you're intentionally discarding the task. This practice is fine, provided you handle potential exceptions within that method to avoid unhandled promise rejections.Runtime/UiServiceMonoComponent.cs.meta (1)
1-11: No issues with meta file.
This.metafile is standard for Unity’s asset/database tracking.CHANGELOG.md (2)
7-8: LGTM!The version header follows the Keep a Changelog format and semantic versioning correctly.
14-16: LGTM!The entry clearly documents the breaking change of renaming
UiPresenterData<T>toUiPresenter<T>.
| private float GetSize(int axis, Vector2 resolution) | ||
| { | ||
| var minSize = LayoutUtility.GetMinSize(RectTransform, axis); | ||
| var maxSize = LayoutUtility.GetFlexibleSize(RectTransform, axis); | ||
| var resolutionX = resolution.x - _padding.left - _padding.right; | ||
| var resolutionY = resolution.y - _padding.top - _padding.bottom; | ||
|
|
||
| if (axis == 0) | ||
| { | ||
| if(resolutionX < minSize && minSize > 0) return minSize; | ||
| if(resolutionX >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | ||
|
|
||
| return RectTransform.sizeDelta.x > resolutionX && resolutionX > 0 ? resolutionX : RectTransform.sizeDelta.x; | ||
| } | ||
|
|
||
| if(resolutionY < minSize && minSize > 0) return minSize; | ||
| if(resolutionY >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | ||
|
|
||
| return RectTransform.sizeDelta.y > resolutionY && resolutionY > 0 ? resolutionY : RectTransform.sizeDelta.y; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate padding vs. resolution constraints.
In GetSize, the padding applied could exceed the resolution if misconfigured, potentially yielding negative or unusable values. Consider clamping the final size to a minimum of zero or providing user feedback if the sum of _padding.left + _padding.right exceeds resolution.x (and similarly for y).
- var resolutionX = resolution.x - _padding.left - _padding.right;
- var resolutionY = resolution.y - _padding.top - _padding.bottom;
+ var resolutionX = Mathf.Max(0, resolution.x - _padding.left - _padding.right);
+ var resolutionY = Mathf.Max(0, resolution.y - _padding.top - _padding.bottom);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private float GetSize(int axis, Vector2 resolution) | |
| { | |
| var minSize = LayoutUtility.GetMinSize(RectTransform, axis); | |
| var maxSize = LayoutUtility.GetFlexibleSize(RectTransform, axis); | |
| var resolutionX = resolution.x - _padding.left - _padding.right; | |
| var resolutionY = resolution.y - _padding.top - _padding.bottom; | |
| if (axis == 0) | |
| { | |
| if(resolutionX < minSize && minSize > 0) return minSize; | |
| if(resolutionX >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | |
| return RectTransform.sizeDelta.x > resolutionX && resolutionX > 0 ? resolutionX : RectTransform.sizeDelta.x; | |
| } | |
| if(resolutionY < minSize && minSize > 0) return minSize; | |
| if(resolutionY >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | |
| return RectTransform.sizeDelta.y > resolutionY && resolutionY > 0 ? resolutionY : RectTransform.sizeDelta.y; | |
| } | |
| private float GetSize(int axis, Vector2 resolution) | |
| { | |
| var minSize = LayoutUtility.GetMinSize(RectTransform, axis); | |
| var maxSize = LayoutUtility.GetFlexibleSize(RectTransform, axis); | |
| var resolutionX = Mathf.Max(0, resolution.x - _padding.left - _padding.right); | |
| var resolutionY = Mathf.Max(0, resolution.y - _padding.top - _padding.bottom); | |
| if (axis == 0) | |
| { | |
| if(resolutionX < minSize && minSize > 0) return minSize; | |
| if(resolutionX >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | |
| return RectTransform.sizeDelta.x > resolutionX && resolutionX > 0 ? resolutionX : RectTransform.sizeDelta.x; | |
| } | |
| if(resolutionY < minSize && minSize > 0) return minSize; | |
| if(resolutionY >= maxSize && maxSize > minSize && maxSize > 1) return maxSize; | |
| return RectTransform.sizeDelta.y > resolutionY && resolutionY > 0 ? resolutionY : RectTransform.sizeDelta.y; | |
| } |
New:
Changed:
Summary by CodeRabbit
Release Notes for Version 0.11.0
New Features
AdjustScreenSizeFitterto enhance Unity's layout sizing capabilitiesImprovements
UiPresenterData<T>toUiPresenter<T>for improved code clarityTechnical Updates
UiServiceMonoComponentto support advanced UI lifecycle management