Release 0.13.2 #23
Release 0.13.2 #23
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several updates across multiple files, primarily focusing on the object pooling system. A new version entry for 0.13.2 is added to the changelog, highlighting the introduction of a constructor for the Changes
Possibly related PRs
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 (
|
|
WalkthroughThe overall change includes various updates and additions to the codebase. Notable changes include the addition of a constructor for custom instantiation in Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (5)
- CHANGELOG.md (6)
- Runtime/ObjectPool.cs (4)
- Tests/Editor/EditMode/CommandServiceTest.cs (1)
- Tests/Editor/EditMode/ObjectPoolTest.cs (2)
- Tests/Editor/EditMode/PoolServiceTest.cs (2)
Files ignored due to filter (1)
- package.json
Review comments generated (11)
- Review: 8
- LGTM: 3
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
|
|
||
| private struct CommandMockup : IGameCommand<IGameLogicMockup> | ||
| private class CommandMockup : IGameCommand<IGameLogicMockup> | ||
| { |
There was a problem hiding this comment.
Changing the CommandMockup struct to a class can have implications for code that relies on value semantics or expects the struct's behavior. Please ensure that this change does not introduce any unintended side effects.
- private struct CommandMockup : IGameCommand<IGameLogicMockup>
+ private class CommandMockup : IGameCommand<IGameLogicMockup>| ## [0.13.2] - 2024-11-13 | ||
|
|
||
| **New**: | ||
| - Added a constructor to *GameObjectPool* that allows to setup a costum instantiator | ||
|
|
||
| **Fixed**: | ||
| - Fixed *ObjectPool* & *PoolService* tests that would block builds sometimes |
There was a problem hiding this comment.
The changelog entry for version 0.13.2 looks good.
| 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.13.2] - 2024-11-13 | ||
|
|
||
| **New**: | ||
| - Added a constructor to *GameObjectPool* that allows to setup a costum instantiator | ||
|
|
||
| **Fixed**: | ||
| - Fixed *ObjectPool* & *PoolService* tests that would block builds sometimes | ||
|
|
||
| ## [0.13.1] - 2024-11-04 | ||
|
|
||
| **Fixed**: |
There was a problem hiding this comment.
Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [22-25]
The changelog entries for versions 0.12.0, 0.11.0, 0.10.0, 0.9.0, 0.8.1, and 0.8.0 are fine.
| **New**: | ||
| - Added *IRngData* to *PoolService* to suppprt read only data structure and allow abtract injection of data into other objects | ||
|
|
||
| **Changed**: | ||
| - Changed RngData to a class in orther to avoid boxing/unboxing performance when injecting IRngData. | ||
| - Changed *RngData* to a class in orther to avoid boxing/unboxing performance when injecting *IRngData*. | ||
|
|
||
| ## [0.11.0] - 2024-10-19 | ||
|
|
||
| - Added Spawn<T>(T data) method to PoolService to allow spawning new objects with defined spawning data | ||
| - Added GetPool<T>() && TryGetPool<T>() methods to PoolService to allow requesting the pool object maintained by the pool service. | ||
| **New**: | ||
| - Added *Spawn<T>(T data)* method to *PoolService* to allow spawning new objects with defined spawning data | ||
| - Added *GetPool<T>()* && *TryGetPool<T>()* methods to *PoolService* to allow requesting the pool object maintained by the pool service. | ||
|
|
||
| **Changed**: | ||
| - Removed IsSpawned<T>() method from PoolService because is not a fundamental function and can now be accessed from the Pool requested from GetPool() | ||
| - Now Spawn<T>(T data) also invokes OnSpawn() without data so objects that implement IPoolEntitySpawn have the entire behaviour lifecycle | ||
| - Removed *IsSpawned<T>()* method from *PoolService* because is not a fundamental function and can now be accessed from the Pool requested from *GetPool()* | ||
| - Now *Spawn<T>(T data)* also invokes *OnSpawn()* without data so objects that implement *IPoolEntitySpawn* have the entire behaviour lifecycle | ||
|
|
||
| ## [0.10.0] - 2024-10-11 |
There was a problem hiding this comment.
The changelog entries for versions 0.7.0, 0.6.2, 0.6.1, 0.6.0, 0.4.1, 0.4.0, 0.3.1, 0.3.0, 0.2.0, 0.1.1, and 0.1.0 are correct.
Overall, the changelog updates seem to accurately reflect the changes made in each version.
Runtime/ObjectPool.cs
No changes to review.
Tests/Editor/EditMode/CommandServiceTest.cs
No changes to review.
Tests/Editor/EditMode/ObjectPoolTest.cs
No changes to review.
Tests/Editor/EditMode/PoolServiceTest.cs
No changes to review.
|
|
||
| /// <summary> | ||
| /// This interface allows to self despawn by maintaining the reference of the pool that created it | ||
| /// This interface allows to self despawn by maintaining the reference of the despawing call |
There was a problem hiding this comment.
The comment mentions "despawing call" instead of "pool that created it". Please update the comment to reflect the correct functionality.
- /// This interface allows to self despawn by maintaining the reference of the despawing call
+ /// This interface allows to self despawn by maintaining the reference of the pool that created it| return false; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter) | ||
| { | ||
| var despawned = false; | ||
|
|
||
| for (var i = 0; i < SpawnedEntities.Count; i++) | ||
| { | ||
| if (!entityGetter(SpawnedEntities[i])) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| despawned = Despawn(SpawnedEntities[i]); | ||
|
|
||
| if (onlyFirst) | ||
| { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return despawned; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public List<T> Clear() | ||
| { |
There was a problem hiding this comment.
The code has been simplified and refactored. It seems that the Despawn method has been moved to a different location in the class, and the logic for clearing the list of spawned entities has been extracted into a separate method called Clear. This change improves modularity and readability. No issues found.
- public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter)
- {
- var despawned = false;
-
- for (var i = 0; i < SpawnedEntities.Count; i++)
- {
- if (!entityGetter(SpawnedEntities[i]))
- {
- continue;
- }
-
- despawned = Despawn(SpawnedEntities[i]);
-
- if (onlyFirst)
- {
- break;
- }
- }
-
- return despawned;
- }
+ /// <inheritdoc />
+ public List<T> Clear()
+ {
+ // implementation
+ }| /// <inheritdoc /> | ||
| public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter) | ||
| { | ||
| var despawned = false; | ||
|
|
||
| for (var i = 0; i < SpawnedEntities.Count; i++) | ||
| { | ||
| if (!entityGetter(SpawnedEntities[i])) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| despawned = Despawn(SpawnedEntities[i]); | ||
|
|
||
| if (onlyFirst) | ||
| { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return despawned; | ||
| } |
There was a problem hiding this comment.
The Despawn method has been updated with a new implementation. It now takes an additional parameter onlyFirst, which indicates whether only the first matching entity should be despawned. The logic for despawning entities based on the provided condition has been refactored. This change improves flexibility and performance. No issues found.
- public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter)
- {
- var despawned = false;
-
- for (var i = 0; i < SpawnedEntities.Count; i++)
- {
- if (!entityGetter(SpawnedEntities[i]))
- {
- continue;
- }
-
- despawned = Despawn(SpawnedEntities[i]);
-
- if (onlyFirst)
- {
- break;
- }
- }
-
- return despawned;
- }
+ /// <inheritdoc />
+ public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter)
+ {
+ // updated implementation
+ }| public GameObjectPool(uint initSize, GameObject sampleEntity, Func<GameObject, GameObject> instantiator) : base(initSize, sampleEntity, instantiator) | ||
| { | ||
| } | ||
|
|
There was a problem hiding this comment.
A new constructor has been added to the GameObjectPool class. It accepts three parameters: initSize, sampleEntity, and instantiator. This change allows customization of the instantiator function used by the pool. No issues found.
- public GameObjectPool(uint initSize, GameObject sampleEntity, Func<GameObject, GameObject> instantiator) : base(initSize, sampleEntity, instantiator)
- {
- }
+ public GameObjectPool(uint initSize, GameObject sampleEntity, Func<GameObject, GameObject> instantiator) : base(initSize, sampleEntity, instantiator)
+ {
+ // implementation
+ }![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
🧹 Outside diff range and nitpick comments (7)
Tests/Editor/EditMode/CommandServiceTest.cs (1)
Line range hint
21-29: LGTM! Consider adding a comment explaining the class choice.The change from struct to class for
CommandMockupaligns well with the service's support for non-struct commands. This is a good change as commands typically maintain state and are passed around, making reference types more appropriate.Consider adding a brief comment explaining why
CommandMockupis implemented as a class rather than a struct, to help future maintainers understand the design decision:+// Implemented as a class to align with CommandService's support for non-struct commands private class CommandMockup : IGameCommand<IGameLogicMockup>Tests/Editor/EditMode/ObjectPoolTest.cs (1)
80-91: Resolve interface issue to enable EntityDespawn testThe commented test case appears to be blocking important verification of the EntityDespawn functionality. The test structure looks correct, using proper mocking and assertions.
I can help investigate and fix the interface issue. Would you like me to:
- Analyze the interface compatibility issue?
- Propose potential fixes?
- Create a GitHub issue to track this?
Please let me know which option you prefer.
CHANGELOG.md (2)
7-14: Fix typos in the new version entryThere are spelling and grammar issues in the changelog entry:
-Added a constructor to *GameObjectPool* that allows to setup a costum instantiator +Added a constructor to *GameObjectPool* that allows to set up a custom instantiator🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ctor to GameObjectPool that allows to setup a costum instantiatorFixed:
- ...
(NOUN_VERB_CONFUSION)
Line range hint
38-160: Fix typos and formatting in historical entriesThere are several spelling issues in the historical entries:
-Added *IRngData* to *PoolService* to suppprt read only data structure and allow abtract injection of data into other objects +Added *IRngData* to *PoolService* to support read only data structure and allow abstract injection of data into other objects -Changed *RngData* to a class in orther to avoid boxing/unboxing performance when injecting *IRngData*. +Changed *RngData* to a class in order to avoid boxing/unboxing performance when injecting *IRngData*.Also, ensure consistent formatting of emphasis markers throughout the changelog (no spaces inside asterisks).
Runtime/ObjectPool.cs (3)
47-47: Fix typo in documentationThere's a typo in the interface documentation: "despawing" should be "despawning".
- /// This interface allows to self despawn by maintaining the reference of the despawing call + /// This interface allows to self despawn by maintaining the reference of the despawning call
242-263: Add parameter documentationThe method implementation looks good, but the XML documentation is missing details about the
entityGetterparameter.Add parameter documentation:
/// <summary> /// Despawns the entity that is valid with the given <paramref name="entityGetter"/> condition and returns it back to /// the pool to be used again later. /// If the given <paramref name="onlyFirst"/> is true then will only despawn one entity and not find more entities /// that match the given <paramref name="entityGetter"/> condition. /// This function does not reset the entity. For that, have the entity implement <see cref="IPoolEntityDespawn"/> /// or do it externally. /// Returns true if was able to despawn the entity back to the pool successfully, false otherwise /// </summary> + /// <param name="onlyFirst">If true, only the first matching entity will be despawned</param> + /// <param name="entityGetter">A function that returns true for entities that should be despawned</param> public bool Despawn(bool onlyFirst, Func<T, bool> entityGetter)
359-361: Add constructor documentationThe new constructor needs XML documentation to explain its purpose and parameters.
Add constructor documentation:
+ /// <summary> + /// Initializes a new instance of the GameObjectPool with a custom instantiator + /// </summary> + /// <param name="initSize">The initial size of the pool</param> + /// <param name="sampleEntity">The sample GameObject to use as a template</param> + /// <param name="instantiator">A custom function to instantiate new GameObjects</param> public GameObjectPool(uint initSize, GameObject sampleEntity, Func<GameObject, GameObject> instantiator) : base(initSize, sampleEntity, instantiator) { }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
CHANGELOG.md(6 hunks)Runtime/ObjectPool.cs(3 hunks)Tests/Editor/EditMode/CommandServiceTest.cs(1 hunks)Tests/Editor/EditMode/ObjectPoolTest.cs(2 hunks)Tests/Editor/EditMode/PoolServiceTest.cs(2 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- Tests/Editor/EditMode/PoolServiceTest.cs
- package.json
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ctor to GameObjectPool that allows to setup a costum instantiator
Fixed:
- ...
(NOUN_VERB_CONFUSION)
🪛 Markdownlint
CHANGELOG.md
104-104: null
Spaces inside emphasis markers
(MD037, no-space-in-emphasis)
🔇 Additional comments (2)
Tests/Editor/EditMode/ObjectPoolTest.cs (2)
18-18: LGTM! Improved type safety in IMockEntity interface
The change from IPoolEntityObject<object> to IPoolEntityObject<IMockEntity> enhances type safety while maintaining flexibility with IPoolEntitySpawn<object>.
80-91: Verify test coverage for MockEntity
While the EntityDespawn test is commented out, we should ensure the MockEntity implementation is covered by other tests.
| public class MockEntity : IMockEntity | ||
| { | ||
| private IObjectPool<IMockEntity> _pool; | ||
|
|
||
| public void Init(IObjectPool<IMockEntity> pool) => _pool = pool; | ||
|
|
||
| public bool Despawn() => _pool.Despawn(this); | ||
| public void OnDespawn() {} | ||
|
|
||
| public void OnSpawn() {} | ||
| public void OnSpawn(object data) {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for _pool in Despawn method
The MockEntity implementation looks good overall, but the Despawn method should validate that _pool has been initialized to prevent NullReferenceException.
Consider adding a null check:
public class MockEntity : IMockEntity
{
private IObjectPool<IMockEntity> _pool;
public void Init(IObjectPool<IMockEntity> pool) => _pool = pool;
- public bool Despawn() => _pool.Despawn(this);
+ public bool Despawn()
+ {
+ if (_pool == null)
+ {
+ throw new InvalidOperationException("Pool not initialized. Call Init before Despawn.");
+ }
+ return _pool.Despawn(this);
+ }
public void OnDespawn() {}
public void OnSpawn() {}
public void OnSpawn(object data) {}
}📝 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.
| public class MockEntity : IMockEntity | |
| { | |
| private IObjectPool<IMockEntity> _pool; | |
| public void Init(IObjectPool<IMockEntity> pool) => _pool = pool; | |
| public bool Despawn() => _pool.Despawn(this); | |
| public void OnDespawn() {} | |
| public void OnSpawn() {} | |
| public void OnSpawn(object data) {} | |
| } | |
| public class MockEntity : IMockEntity | |
| { | |
| private IObjectPool<IMockEntity> _pool; | |
| public void Init(IObjectPool<IMockEntity> pool) => _pool = pool; | |
| public bool Despawn() | |
| { | |
| if (_pool == null) | |
| { | |
| throw new InvalidOperationException("Pool not initialized. Call Init before Despawn."); | |
| } | |
| return _pool.Despawn(this); | |
| } | |
| public void OnDespawn() {} | |
| public void OnSpawn() {} | |
| public void OnSpawn(object data) {} | |
| } |
New:
Fixed:
Summary by CodeRabbit
GameObjectPoolfor custom instantiation.ObjectPoolandPoolServicethat caused intermittent build failures.ObjectPool.cs.Despawnmethod within theObjectPoolclass.CommandMockupto a class in theIGameLogicMockupinterface, which may impact code utilizing this interface.IMockEntityinterface and added a newMockEntityclass implementing it.MockEntityclass.Summary by CodeRabbit
Release Notes for Version 0.13.2
New Features
Bug Fixes
Documentation
Tests