Merge pull request #31 from CoderGamester/develop

Release 2.0.1

Author: CoderGamester

.gitignore

@@ -79,3 +79,6 @@ crashlytics-build.properties
 
 
 
+
+# Tests audit history (unity-tests-audit skill -- local developer state, never committed)
+.audit-history.md

AGENTS.md

@@ -128,7 +128,7 @@ flowchart TD
 
 ### Build/Version Info
 `Runtime/VersionServices.cs`
-- Static class for `version-data` Resources metadata. `VersionExternal` is always safe; `VersionInternal`, `Branch`, `Commit`, and `BuildNumber` require successful `LoadVersionDataAsync()` first.
+- Static class for `version-data` Resources metadata. `VersionExternal` is always safe; `VersionInternal`, `Branch`, `Commit`, and `BuildNumber` require either `LoadVersionData()` (sync) or `LoadVersionDataAsync()` (async) to have completed successfully first.
 
 ## 3. Key Directories / Files
 
@@ -243,7 +243,9 @@ The concrete `PoolService` stays in `Runtime/` root under `GameLovers.Services`
 - `VersionEditorUtils` writes `version-data.txt` on every domain reload (`[InitializeOnLoadMethod]`) and can be invoked by build pipelines. It uses git CLI; failures are handled gracefully.
 - The **write folder** is configurable per-project via `VersioningEditorSettings.instance.ResourcesFolderPath` (default `Assets/Configs/Resources`). Change it from the Versioning tab in the Services Explorer (browse + reset). The chosen folder must contain a `Resources` path segment so `Resources.Load<TextAsset>("version-data")` can locate the file at runtime.
 - `VersioningEditorSettings` persists to `ProjectSettings/VersioningEditorSettings.asset` (editor-only, not committed to version control by default).
-- `VersionExternal` is always safe (reads `Application.version` directly). `VersionInternal`, `Branch`, `Commit`, and `BuildNumber` throw `Exception("Version Data not loaded.")` if data has not been loaded — call `LoadVersionDataAsync()` early in boot.
+- `VersionExternal` is always safe (reads `Application.version` directly). `VersionInternal`, `Branch`, `Commit`, and `BuildNumber` throw `Exception("Version Data not loaded.")` if data has not been loaded — call `LoadVersionData()` (sync) or `LoadVersionDataAsync()` (async) early in boot.
+- **Sync vs async load**: `LoadVersionData()` uses `Resources.Load<TextAsset>` (synchronous, main-thread); `LoadVersionDataAsync()` uses `Resources.LoadAsync<TextAsset>` wrapped in a `TaskCompletionSource`. Both funnel into the private `ApplyTextAsset(TextAsset, bool asyncContext)` helper that parses JSON, flips `_loaded`, and calls `Resources.UnloadAsset`. Behaviour is identical apart from the wording in the failure log line (`"Could not async load …"` vs `"Could not load …"`). Sync is the recommended default for the shipping `version-data.txt` (a few hundred bytes); async is only worth the ceremony if `VersionData` is extended with large embedded blobs (e.g. a baked manifest) that would noticeably stall the main thread.
+- `VersionServices.IsOutdatedVersion(string)` requires a 3-part `Major.Minor.Patch` semver and throws `IndexOutOfRangeException` on any 1- or 2-part input — the parser unconditionally accesses `Split('.')[0..2]`. Consumers that compare against `Application.version` must ensure `ProjectSettings.bundleVersion` is 3-part (defaults to `0.1` / `1.0` for fresh projects, which will throw). Either bump `bundleVersion` to a 3-part value, guard with `parts.Length < 3` at the call site, or harden the method itself with a length guard. Tests calling this against the host editor's `Application.version` should `Assert.Inconclusive` on shorter strings rather than `Assert.Throws` — the throw is a real production bug surface, not a documented contract.
 
 ### Editor Introspection (InternalsVisibleTo)
 

CHANGELOG.md

@@ -4,6 +4,15 @@ 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).
 
+## [2.0.1] - 2026-05-20
+
+**New**
+- Added new test suite for more rebust code coverage
+- Added `VersionServices.LoadVersionData()` — synchronous sibling of `LoadVersionDataAsync()` for consumers who want to populate version metadata at boot without an `await`. Both methods now funnel into a shared private `ApplyTextAsset` helper, so behaviour is identical. Sync is the recommended default for the shipping `version-data.txt` (a few hundred bytes); async remains available for cases where `VersionData` is extended with large embedded blobs. Covered by `Tests/EditMode/Unit/VersionServicesSyncLoadTest.cs`.
+
+**Fixed**:
+- `Tests/AGENTS.md` §8 extended with a new **"Authorized reflection sites (storage-assertion exception)"** subsection.
+
 ## [2.0.0] - 2026-04-26
 
 **New**:
@@ -19,8 +28,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added `AssetResolverService` (implements `IAssetResolverService` / `IAssetAdderService`) to `Runtime/` root (ns `GameLovers.Services`)
 - Added Editor/AssetsImporter/: `AssetsImporter`, `AssetsToolImporter`, `AssetConfigsImporter`, `AddressableIdsGenerator`, `AddressablesIdGeneratorSettings` (ns `GameLovers.Services.AssetsImporter.Editor`)
 - Added importable **Samples** under `Samples~/` (importable via Unity Package Manager > GameLovers Services > Samples).
-  - **Services Playground** — single-scene, zero-setup walk-through that wires every foundation service via `MainInstaller` and exercises 10 of 13 Services Explorer tabs end-to-end. 
-  - **Asset Resolver** — focused demo of `AssetResolverService` end-to-end (`AddConfigs` / `RequestAsset` / `UnloadAssets`) with `SpriteConfigs : AssetConfigsScriptableObject<SpriteId, Sprite>`. Drives the three Services Explorer tabs the Playground does not cover (Asset Resolver, Assets Importer, Addressable Ids). 
+  - **Services Playground** — single-scene, zero-setup walk-through that wires every foundation service via `MainInstaller` and exercises 10 of 13 Services Explorer tabs end-to-end.
+  - **Asset Resolver** — focused demo of `AssetResolverService` end-to-end (`AddConfigs` / `RequestAsset` / `UnloadAssets`) with `SpriteConfigs : AssetConfigsScriptableObject<SpriteId, Sprite>`. Drives the three Services Explorer tabs the Playground does not cover (Asset Resolver, Assets Importer, Addressable Ids).
 
 **Changed**:
 - Addressable Ids generator and Assets Importer settings moved from `Assets/*.asset` ScriptableObjects to `ProjectSettings/` ScriptableSingletons (mirrors `VersioningEditorSettings`).
@@ -41,7 +50,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - `AddressablesAssetLoader.UnloadAsset` no longer calls `GC.Collect()`, `GC.WaitForPendingFinalizers()`, or `Resources.UnloadUnusedAssets()`. The method now only decrements the Addressables reference count. The old implementation caused PlayMode Test Runner crashes on macOS and O(total-assets-in-memory) main-thread stalls per per-asset release. Callers that need memory reclamation should invoke `Resources.UnloadUnusedAssets()` themselves at appropriate moments (scene transitions, boot, memory-pressure events); Unity also runs an unused-assets sweep automatically on `LoadSceneMode.Single` scene loads.
 - Corrected `IAssetLoader.UnloadAsset` XML documentation: removed the incorrect "will also destroy GameObject instances" claim — `Addressables.Release(gameObject)` does not destroy the instance; callers must `Object.Destroy` it separately.
 - `IAsyncCoroutine.StopCoroutine(bool triggerOnComplete)` now honors its `triggerOnComplete` parameter and flips `IsCompleted` to `true` and `IsRunning` to `false` after stopping. The previous implementation always invoked `OnComplete` callbacks regardless of the flag and left state flags unchanged, so consumers could not distinguish a stopped coroutine from a running one and `triggerOnComplete: false` was silently ignored.
-- `GameObjectPool.Dispose()` and `GameObjectPool<T>.Dispose()` now skip pooled entries whose underlying `GameObject` has already been destroyed by an external owner (e.g. a parent GameObject was destroyed while pooled instances were still reparented under it via `DespawnToSampleParent`). 
+- `GameObjectPool.Dispose()` and `GameObjectPool<T>.Dispose()` now skip pooled entries whose underlying `GameObject` has already been destroyed by an external owner (e.g. a parent GameObject was destroyed while pooled instances were still reparented under it via `DespawnToSampleParent`).
 
 **Breaking Changes** — see `MIGRATION.md` for details:
 - Pool types moved from `GameLovers.Services` to `GameLovers.Services.Pooling` (`IPoolService`, `IObjectPool`, `IObjectPool<T>`, `ObjectPool<T>`, `ObjectPoolBase<T>`, `GameObjectPool`, `GameObjectPool<T>`, `IPoolEntitySpawn`, `IPoolEntitySpawn<T>`, `IPoolEntityDespawn`, `IPoolEntityObject<T>`). `PoolService` concrete class remains in `GameLovers.Services`.
@@ -71,7 +80,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 **Fixed**:
 - Fixed the *README.md* file to now follow best practices in OSS standards for Unity's package library projects
 - Fixed linter warnings in *VersionServices.cs* (redundant field initialization, unused lambda parameter, member shadowing)
-- Fixed *GameObjectPool* not invoking *IPoolEntitySpawn.OnSpawn()* and *IPoolEntityDespawn.OnDespawn()* on components attached to spawned GameObjects. 
+- Fixed *GameObjectPool* not invoking *IPoolEntitySpawn.OnSpawn()* and *IPoolEntityDespawn.OnDespawn()* on components attached to spawned GameObjects.
 
 ## [0.15.1] - 2025-09-24
 
@@ -84,7 +93,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Added *StartDelayCall* method to *ICoroutineService* to allow deferred methods to be safely executed within the bounds of a Unity Coroutine
 - Added the possibility to know the current state of an *IAsyncCoroutine*
 - Added the access to the Sample Entity used to generete new entites within an *IObjectPool<T>* and destroy it when disposing the object pool
-- Added the possibility to reset an *IObjectPool<T>* to a new state 
+- Added the possibility to reset an *IObjectPool<T>* to a new state
 
 ## [0.14.1] - 2024-11-30
 
@@ -219,7 +228,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 **Changed**:
 - Renamed *IDataWriter* and it's *FlushData* methods to *IDataSaver* & *SaveData* respectively to match with it's execution logic scope
-- Moved the *AddData* to the *IDataService* to allow the *IDataSaver* have the single responsibility of saving data into disk  
+- Moved the *AddData* to the *IDataService* to allow the *IDataSaver* have the single responsibility of saving data into disk
 
 ## [0.4.1] - 2020-07-09
 
@@ -243,7 +252,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Now the *MainInstaller* checks the object binding relationship in compile time
 - Improved the *ObjectPools* helper classes with a now static global instatiator for game objects.
 - Now the *PoolService* is only a service container for objects pools and no longer creates/initializes new pools.
-- Removed *Pool.Clear* functionality. Use *DespawnAll* or delete the pool instead 
+- Removed *Pool.Clear* functionality. Use *DespawnAll* or delete the pool instead
 
 **Fixed**:
 - The *CoroutineService* no longer fails on null coroutines

README.md

@@ -232,10 +232,13 @@ cmd.ExecuteCommand(new LevelUpCommand());
 ### Version Services
 
 ```csharp
-await VersionServices.LoadVersionDataAsync();
+// Pick one of:
+VersionServices.LoadVersionData();          // sync — recommended for tiny version-data.txt payloads
+await VersionServices.LoadVersionDataAsync(); // async — use if VersionData embeds large blobs
+
 string branch = VersionServices.Branch;
 string commit = VersionServices.Commit;
-string ext    = VersionServices.VersionExternal; // always safe, no await needed
+string ext    = VersionServices.VersionExternal; // always safe, no load needed
 ```
 
 ### Asset Loading

Runtime/VersionServices.cs

@@ -55,6 +55,27 @@ public struct VersionData
 		private static VersionData _versionData;
 		private static bool _loaded;
 
+		/// <summary>
+		/// Load the internal version string from resources synchronously. Should be called once
+		/// when the app is started. Intended for tiny payloads (the default <c>version-data.txt</c>
+		/// is a few hundred bytes); use <see cref="LoadVersionDataAsync"/> if <see cref="VersionData"/>
+		/// is extended with large blobs that would noticeably stall the main thread.
+		/// </summary>
+		public static void LoadVersionData()
+		{
+			try
+			{
+				var textAsset = Resources.Load<TextAsset>(VersionDataFilename);
+
+				ApplyTextAsset(textAsset, asyncContext: false);
+			}
+			catch (Exception e)
+			{
+				Debug.LogError($"Error loading version data: {e.Message}");
+				_loaded = false;
+			}
+		}
+
 		/// <summary>
 		/// Load the internal version string from resources async. Should be called once when the
 		/// app is started.
@@ -70,17 +91,7 @@ public static async Task LoadVersionDataAsync()
 
 				var textAsset = await source.Task;
 
-				if (!textAsset)
-				{
-					Debug.LogError("Could not async load version data from Resources.");
-					_loaded = false;
-					return;
-				}
-
-				_versionData = JsonUtility.FromJson<VersionData>(textAsset.text);
-				_loaded = true;
-
-				Resources.UnloadAsset(textAsset);
+				ApplyTextAsset(textAsset, asyncContext: true);
 			}
 			catch (Exception e)
 			{
@@ -89,6 +100,21 @@ public static async Task LoadVersionDataAsync()
 			}
 		}
 
+		private static void ApplyTextAsset(TextAsset textAsset, bool asyncContext)
+		{
+			if (!textAsset)
+			{
+				Debug.LogError($"Could not {(asyncContext ? "async " : string.Empty)}load version data from Resources.");
+				_loaded = false;
+				return;
+			}
+
+			_versionData = JsonUtility.FromJson<VersionData>(textAsset.text);
+			_loaded = true;
+
+			Resources.UnloadAsset(textAsset);
+		}
+
 		/// <summary>
 		/// Requests to check if the provided version is newer compared to the local app version
 		/// </summary>

Tests/AGENTS.md

@@ -67,6 +67,12 @@ Why both keys: `RunSettings.Instance` is a lazy-loaded singleton (`ResourcesLoad
 - Use `[Order(n)]` when tests must run in sequence (e.g., `VersionServicesIntegrationTest` resets static state, then loads, then reads).
 - Reset shared static state in `[SetUp]` (reflection into private fields is acceptable for static classes like `VersionServices`).
 
+### Authorized reflection sites (storage-assertion exception)
+Reflection on private state is also authorized when a setter has no observable readback path through the public API and exercising the side-effect would require a runtime environment the EditMode harness cannot provide (e.g., a partially-loaded `AssetReference`). In those cases, asserting the storage field directly via `BindingFlags.NonPublic | BindingFlags.Instance` is acceptable and preferable to a Red-testability skip. The test method MUST be a single setter-storage assertion (not a multi-step behavioural assertion); if behaviour is what you need to verify, refactor to expose an `internal` accessor under `InternalsVisibleTo` instead.
+
+Currently authorized:
+- `AssetResolverServiceTest.AddDebugConfigs_StoresAllProvided` — reads the private `AssetResolverService._errorMaterial` field to confirm `AddDebugConfigs` stored its argument. The fallback-material lookup path (`AssetResolverService.Convert<T>` at `Runtime/AssetResolverService.cs:474`) only fires when `!assetReference.IsDone`, which the EditMode harness cannot fabricate without a real Addressables catalog. Documented here per the Type B audit run on 2026-05-04 (Referee §4 missed-anti-pattern finding, parent picked option A).
+
 ## 9. Test Directory Layout
 
 | Directory | Contents |

Tests/EditMode/Unit/AddressableIdsEditorSettingsTest.cs

@@ -8,6 +8,28 @@ namespace GameLoversEditor.Services.Tests
 	[TestFixture]
 	public class AddressableIdsEditorSettingsTest
 	{
+		// AddressableIdsEditorSettings is a ScriptableSingleton persisted to ProjectSettings/;
+		// snapshot + restore so test mutations don't leak into the user's project.
+		private string _originalScriptFilename;
+		private string _originalNamespace;
+		private string _originalAddressableLabel;
+
+		[SetUp]
+		public void SaveOriginalSettings()
+		{
+			_originalScriptFilename = AddressableIdsEditorSettings.instance.ScriptFilename;
+			_originalNamespace = AddressableIdsEditorSettings.instance.Namespace;
+			_originalAddressableLabel = AddressableIdsEditorSettings.instance.AddressableLabel;
+		}
+
+		[TearDown]
+		public void RestoreOriginalSettings()
+		{
+			AddressableIdsEditorSettings.instance.ScriptFilename = _originalScriptFilename;
+			AddressableIdsEditorSettings.instance.Namespace = _originalNamespace;
+			AddressableIdsEditorSettings.instance.AddressableLabel = _originalAddressableLabel;
+		}
+
 		// ---- IsValidIdentifier ----
 
 		[Test]
@@ -116,5 +138,43 @@ public void IsValidNamespace_DeepHierarchy_ReturnsTrue()
 			Assert.IsTrue(AddressableIdsEditorSettings.IsValidNamespace("Com.GameLovers.Game.Ids", out var error));
 			Assert.IsNull(error);
 		}
+
+		// ---- Setter normalization ----
+
+		[Test]
+		public void ScriptFilename_SetterNormalizesAndPersists()
+		{
+			AddressableIdsEditorSettings.instance.ScriptFilename = "  CustomFilename  ";
+
+			Assert.AreEqual("CustomFilename", AddressableIdsEditorSettings.instance.ScriptFilename);
+
+			AddressableIdsEditorSettings.instance.ScriptFilename = null;
+
+			Assert.AreEqual("AddressableId", AddressableIdsEditorSettings.instance.ScriptFilename);
+		}
+
+		[Test]
+		public void Namespace_SetterNormalizesAndPersists()
+		{
+			AddressableIdsEditorSettings.instance.Namespace = "  Custom.Namespace  ";
+
+			Assert.AreEqual("Custom.Namespace", AddressableIdsEditorSettings.instance.Namespace);
+
+			AddressableIdsEditorSettings.instance.Namespace = null;
+
+			Assert.AreEqual("Game.Ids", AddressableIdsEditorSettings.instance.Namespace);
+		}
+
+		[Test]
+		public void AddressableLabel_SetterNormalizesAndPersists()
+		{
+			AddressableIdsEditorSettings.instance.AddressableLabel = "  custom-label  ";
+
+			Assert.AreEqual("custom-label", AddressableIdsEditorSettings.instance.AddressableLabel);
+
+			AddressableIdsEditorSettings.instance.AddressableLabel = null;
+
+			Assert.AreEqual(string.Empty, AddressableIdsEditorSettings.instance.AddressableLabel);
+		}
 	}
 }

Tests/EditMode/Unit/AssetConfigsScriptableObjectTest.cs

@@ -0,0 +1,40 @@
+using System.Collections.Generic;
+using GameLovers.GameData;
+using GameLovers.Services.AssetsImporter;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.AddressableAssets;
+
+// ReSharper disable once CheckNamespace
+
+namespace GameLoversEditor.Services.Tests
+{
+	[TestFixture]
+	public class AssetConfigsScriptableObjectTest
+	{
+		private class TestAssetConfigs : AssetConfigsScriptableObject<int, Sprite> { }
+
+		[Test]
+		public void OnAfterDeserialize_RebuildsDictionaryFromConfigs()
+		{
+			var so = ScriptableObject.CreateInstance<TestAssetConfigs>();
+
+			var refA = new AssetReference();
+			var refB = new AssetReference();
+			so.Configs = new List<Pair<int, AssetReference>>
+			{
+				new Pair<int, AssetReference>(1, refA),
+				new Pair<int, AssetReference>(2, refB)
+			};
+
+			so.OnAfterDeserialize();
+
+			Assert.IsNotNull(so.ConfigsDictionary);
+			Assert.AreEqual(2, so.ConfigsDictionary.Count);
+			Assert.AreSame(refA, so.ConfigsDictionary[1]);
+			Assert.AreSame(refB, so.ConfigsDictionary[2]);
+
+			Object.DestroyImmediate(so);
+		}
+	}
+}