Skip to content

Release 0.9.0 #17

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Release 0.9.0 #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ All notable changes to this package will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html)


## [0.9.0] - 2024-11-01

- Added *GetUi<T>* method to the *IUiService*. It requests the *UiPresenter* by directly using generic T
- Added *IsVisible<T>* method to the *IUiService*. It requests the visibility state of *UiPresenter*
- Added IReadOnlyList property *VisiblePresenters* to the *IUiService* to allow external entities to access the list of visible *UiPresenter*

***Changed**:
- Removed *GetAllVisibleUi()* method. Use *IsVisible<T>* method instead

## [0.8.0] - 2024-10-29

- Added new *PresenterDelayerBase*, *AnimationDelayer* and *TimeDelayer* to support presenters that open/close with a delay
Expand Down
28 changes: 22 additions & 6 deletions Runtime/IUiService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ public interface IUiService
/// Gets a read-only dictionary of the Presenters currently Loaded in memory by the UI service.
/// </summary>
IReadOnlyDictionary<Type, UiPresenter> LoadedPresenters { get; }

/// <summary>
/// Gets a read-only list of the Presenters currently currently visible and were opened by the UI service.
/// </summary>
IReadOnlyList<Type> VisiblePresenters { get; }

/// <summary>
/// Gets a read-only dictionary of the layers used by the UI service.
Expand All @@ -28,6 +33,23 @@ public interface IUiService
/// </summary>
IReadOnlyDictionary<int, UiSetConfig> UiSets { get; }

/// <summary>
/// Requests the UI of given type <typeparamref name="T"/>
/// </summary>
/// <exception cref="KeyNotFoundException">
/// Thrown if the service does NOT contain an <see cref="UiPresenter"/> of the given <typeparamref name="T"/>
/// </exception>
/// <typeparam name="T">The type of UI presenter requested.</typeparam>
/// <returns>The UI of type <typeparamref name="T"/> requested</returns>
T GetUi<T>() where T : UiPresenter;

/// <summary>
/// Requests the visible state of the given UI type <typeparamref name="T"/>.
/// </summary>
/// <typeparam name="T">The type of UI presenter to check if is visible or not.</typeparam>
/// <returns>True if the UI is visble, false otherwise</returns>
bool IsVisible<T>() where T : UiPresenter;

/// <summary>
/// Adds a UI configuration to the service.
/// </summary>
Expand Down Expand Up @@ -141,12 +163,6 @@ public interface IUiService
/// <exception cref="KeyNotFoundException">Thrown if the service does not contain a UI set with the specified ID.</exception>
void UnloadUiSet(int setId);

/// <summary>
/// Gets a list of all visible UI presenters.
/// </summary>
/// <returns>A list of types of visible UI presenters.</returns>
List<Type> GetAllVisibleUi();

/// <summary>
/// Opens a UI presenter asynchronously, loading its assets if necessary.
/// </summary>
Expand Down
25 changes: 17 additions & 8 deletions Runtime/UiService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public class UiService : IUiServiceInit
private readonly IDictionary<int, GameObject> _layers = new Dictionary<int, GameObject>();
private readonly IDictionary<Type, UiPresenter> _uiPresenters = new Dictionary<Type, UiPresenter>();

private Type _loadingSpinnerType;
private Transform _uiParent;

/// <inheritdoc />
public IReadOnlyDictionary<Type, UiPresenter> LoadedPresenters => new Dictionary<Type, UiPresenter>(_uiPresenters);

Expand All @@ -28,8 +31,8 @@ public class UiService : IUiServiceInit
/// <inheritdoc />
public IReadOnlyDictionary<int, UiSetConfig> UiSets => new Dictionary<int, UiSetConfig>(_uiSets);

private Type _loadingSpinnerType;
private Transform _uiParent;
/// <inheritdoc />
public IReadOnlyList<Type> VisiblePresenters => new List<Type>(_visibleUiList);
Comment on lines +34 to +35
Copy link

@coderabbitai coderabbitai bot Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider caching the VisiblePresenters list for better performance.

The current implementation creates a new List on every property access. For frequent access patterns, this could impact performance.

Consider caching the list and invalidating it only when the visible presenters change:

+private IReadOnlyList<Type> _cachedVisiblePresenters;
+private bool _visiblePresentersDirty = true;

-public IReadOnlyList<Type> VisiblePresenters => new List<Type>(_visibleUiList);
+public IReadOnlyList<Type> VisiblePresenters
+{
+    get
+    {
+        if (_visiblePresentersDirty || _cachedVisiblePresenters == null)
+        {
+            _cachedVisiblePresenters = new List<Type>(_visibleUiList);
+            _visiblePresentersDirty = false;
+        }
+        return _cachedVisiblePresenters;
+    }
+}

Then set _visiblePresentersDirty = true in methods that modify _visibleUiList:

  • OpenUi
  • CloseUi
  • CloseAllUi
  • RemoveUi

Committable suggestion skipped: line range outside the PR's diff.


public UiService() : this(new UiAssetLoader()) { }

Expand Down Expand Up @@ -65,6 +68,18 @@ public void Init(UiConfigs configs)
}
}

/// <inheritdoc />
public T GetUi<T>() where T : UiPresenter
{
return _uiPresenters[typeof(T)] as T;
}
Comment on lines +71 to +75
Copy link

@coderabbitai coderabbitai bot Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and XML documentation for GetUi.

The current implementation has potential failure points:

  1. KeyNotFoundException if the presenter doesn't exist
  2. InvalidCastException if T doesn't match the actual presenter type

Consider this safer implementation with proper documentation:

+/// <summary>
+/// Gets a UI presenter of type T if it exists in the loaded presenters.
+/// </summary>
+/// <typeparam name="T">The type of UI presenter to retrieve.</typeparam>
+/// <returns>The UI presenter of type T.</returns>
+/// <exception cref="KeyNotFoundException">Thrown when the UI presenter is not loaded.</exception>
+/// <exception cref="InvalidCastException">Thrown when the loaded presenter doesn't match type T.</exception>
 public T GetUi<T>() where T : UiPresenter
 {
-    return _uiPresenters[typeof(T)] as T;
+    if (!_uiPresenters.TryGetValue(typeof(T), out var presenter))
+    {
+        throw new KeyNotFoundException($"UI presenter of type {typeof(T)} is not loaded.");
+    }
+    
+    if (presenter is not T typedPresenter)
+    {
+        throw new InvalidCastException($"Loaded presenter is of type {presenter.GetType()} which cannot be cast to {typeof(T)}");
+    }
+    
+    return typedPresenter;
 }
📝 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.

Suggested change
/// <inheritdoc />
public T GetUi<T>() where T : UiPresenter
{
return _uiPresenters[typeof(T)] as T;
}
/// <summary>
/// Gets a UI presenter of type T if it exists in the loaded presenters.
/// </summary>
/// <typeparam name="T">The type of UI presenter to retrieve.</typeparam>
/// <returns>The UI presenter of type T.</returns>
/// <exception cref="KeyNotFoundException">Thrown when the UI presenter is not loaded.</exception>
/// <exception cref="InvalidCastException">Thrown when the loaded presenter doesn't match type T.</exception>
public T GetUi<T>() where T : UiPresenter
{
if (!_uiPresenters.TryGetValue(typeof(T), out var presenter))
{
throw new KeyNotFoundException($"UI presenter of type {typeof(T)} is not loaded.");
}
if (presenter is not T typedPresenter)
{
throw new InvalidCastException($"Loaded presenter is of type {presenter.GetType()} which cannot be cast to {typeof(T)}");
}
return typedPresenter;
}


/// <inheritdoc />
public bool IsVisible<T>() where T : UiPresenter
{
return _visibleUiList.Contains(typeof(T));
}

/// <inheritdoc />
public void AddUiConfig(UiConfig config)
{
Expand Down Expand Up @@ -219,12 +234,6 @@ public void UnloadUiSet(int setId)
}
}

/// <inheritdoc />
public List<Type> GetAllVisibleUi()
{
return new List<Type>(_visibleUiList);
}

/// <inheritdoc />
public async Task<UiPresenter> OpenUiAsync(Type type)
{
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"name": "com.gamelovers.uiservice",
"displayName": "UiService",
"author": "Miguel Tomas",
"version": "0.8.0",
"unity": "2022.4",
"version": "0.9.0",
"unity": "2022.3",
"license": "MIT",
"description": "This package provides a service to help manage an Unity's, game UI.\nIt allows to open, close, load, unload and request any Ui Configured in the game.\nThe package provides a Ui Set that allows to group a set of Ui Presenters to help load, open and close multiple Uis at the same time.\n\nTo help configure the game's UI you need to create a UiConfigs Scriptable object by:\n- Right Click on the Project View > Create > ScriptableObjects > Configs > UiConfigs",
"dependencies": {
Expand Down