feat(godot): 添加资源仓储功能支持#46
Merged
Merged
Conversation
- 新增 IHasKey 接口定义键值访问契约 - 新增 IRepository 接口提供通用数据仓储功能 - 实现 GodotResourceRepository 类支持资源的存储和加载 - 添加 IResourceRepository 接口扩展通用仓储功能 - 实现从路径批量加载 Godot 资源的功能 - 支持递归加载子目录中的资源文件 - 提供 .tres 和 .res 文件的自动识别和加载
Reviewer's GuideIntroduces a generic repository abstraction and Godot-specific resource repository implementation based on keyed resources, including recursive loading from filesystem paths, along with a small cleanup in ECS tests. Sequence diagram for GodotResourceRepository LoadFromPath recursive resource loadingsequenceDiagram
actor GameSystem
participant Repo as GodotResourceRepository
participant Dir as DirAccess
participant GD as GD
GameSystem->>Repo: LoadFromPath(paths, recursive)
loop for each path in paths
Repo->>Repo: LoadSinglePath(path, recursive)
Repo->>Dir: Open(path)
alt dir is null
Dir-->>Repo: null
Repo->>GD: PushWarning("Path not found")
else dir is valid
Dir-->>Repo: dir
Repo->>Dir: ListDirBegin()
loop entries
Repo->>Dir: GetNext()
Dir-->>Repo: entry
alt entry is empty
Repo->>Dir: ListDirEnd()
break
end
else entry is directory
Repo->>Dir: CurrentIsDir()
Dir-->>Repo: true
alt recursive and not . or ..
Repo->>Repo: LoadSinglePath(subPath, true)
end
else entry is file
Repo->>Dir: CurrentIsDir()
Dir-->>Repo: false
alt endsWith .tres or .res
Repo->>GD: Load~TResource~(fullPath)
GD-->>Repo: resource
alt resource is null
Repo->>GD: PushWarning("Failed to load resource")
else resource valid
Repo->>Repo: Add(resource.Key, resource)
end
else other extension
Repo-->>Repo: skip file
end
end
end
end
end
Class diagram for new repository and Godot resource repository abstractionsclassDiagram
direction LR
class IHasKey~TKey~ {
<<interface>>
+TKey Key
}
class IRepository~TKey, TValue~ {
<<interface>>
+void Add(TKey key, TValue value)
+TValue Get(TKey key)
+bool TryGet(TKey key, out TValue value)
+IReadOnlyCollection~TValue~ GetAll()
+bool Contains(TKey key)
+void Remove(TKey key)
+void Clear()
}
class IResourceRepository~TKey, TResource~ {
<<interface>>
+void LoadFromPath(IEnumerable~string~ paths, bool recursive)
+void LoadFromPath(bool recursive, string[] paths)
}
class GodotResourceRepository~TKey, TResource~ {
-Dictionary~TKey, TResource~ _storage
+void Add(TKey key, TResource value)
+TResource Get(TKey key)
+bool TryGet(TKey key, out TResource value)
+IReadOnlyCollection~TResource~ GetAll()
+bool Contains(TKey key)
+void Remove(TKey key)
+void Clear()
+void LoadFromPath(IEnumerable~string~ paths, bool recursive)
+void LoadFromPath(bool recursive, string[] paths)
-void LoadSinglePath(string path, bool recursive)
}
class Resource {
}
IResourceRepository~TKey, TResource~ --|> IRepository~TKey, TResource~
GodotResourceRepository~TKey, TResource~ --|> IResourceRepository~TKey, TResource~
TResource --|> Resource
TResource ..|> IHasKey~TKey~
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
GetAll()methods inIRepository/IResourceRepositoryandGodotResourceRepositorycurrently returnIReadOnlyCollection<T>but_storage.Valuesis aDictionary<TKey,TValue>.ValueCollectionwhich does not implementIReadOnlyCollection<T>; consider either changing the return type toICollection<T>or materializing a collection (e.g.ToArray()orToList()) to match the signature. - The two
LoadFromPathoverloads use different parameter orders (IEnumerable<string> paths, bool recursive = falsevsbool recursive = false, params string[] paths), which can be confusing at call sites; consider aligning their parameter order (e.g. alwayspaths, recursive) or removing the default onrecursivefor the params overload for clearer usage. - In
LoadSinglePath,Add(resource.Key, resource)will throw on duplicate keys and abort the whole loading process; if duplicate keys are expected or possible in content, consider handling this more gracefully (e.g. skip and log a warning, or allow overwrite) instead of throwing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `GetAll()` methods in `IRepository`/`IResourceRepository` and `GodotResourceRepository` currently return `IReadOnlyCollection<T>` but `_storage.Values` is a `Dictionary<TKey,TValue>.ValueCollection` which does not implement `IReadOnlyCollection<T>`; consider either changing the return type to `ICollection<T>` or materializing a collection (e.g. `ToArray()` or `ToList()`) to match the signature.
- The two `LoadFromPath` overloads use different parameter orders (`IEnumerable<string> paths, bool recursive = false` vs `bool recursive = false, params string[] paths`), which can be confusing at call sites; consider aligning their parameter order (e.g. always `paths, recursive`) or removing the default on `recursive` for the params overload for clearer usage.
- In `LoadSinglePath`, `Add(resource.Key, resource)` will throw on duplicate keys and abort the whole loading process; if duplicate keys are expected or possible in content, consider handling this more gracefully (e.g. skip and log a warning, or allow overwrite) instead of throwing.
## Individual Comments
### Comment 1
<location> `GFramework.Godot/data/GodotResourceRepository.cs:121-123` </location>
<code_context>
+ /// </summary>
+ /// <param name="recursive">是否递归加载子目录中的资源</param>
+ /// <param name="paths">资源文件路径的参数数组</param>
+ public void LoadFromPath(bool recursive = false, params string[] paths)
+ {
+ LoadFromPath(paths, recursive);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** The overload with a defaulted `recursive` parameter may be confusing at call sites and could be simplified.
Because `recursive` precedes the `params` array, callers can’t actually omit it positionally despite the default value; they must pass a `bool` or use a named argument. This can make call sites harder to read. Consider either requiring `recursive` to be passed explicitly on this overload, or splitting into clearer methods (e.g. `LoadFromPath(params string[] paths)` and `LoadFromPathRecursive(params string[] paths)`) so the API intent is unambiguous.
Suggested implementation:
```csharp
/// <summary>
/// 从指定路径数组加载资源到仓储中（非递归）。
/// </summary>
/// <param name="paths">资源文件路径的参数数组</param>
public void LoadFromPath(params string[] paths)
{
LoadFromPath(paths, recursive: false);
}
/// <summary>
/// 从指定路径数组递归加载资源到仓储中。
/// </summary>
/// <param name="paths">资源文件路径的参数数组</param>
public void LoadFromPathRecursive(params string[] paths)
{
LoadFromPath(paths, recursive: true);
}
```
1. Ensure there is an existing overload `LoadFromPath(IEnumerable<string> paths, bool recursive)` or `LoadFromPath(string[] paths, bool recursive)` that the new methods can delegate to. If its signature differs, adjust the delegation calls accordingly.
2. Update any existing call sites that relied on the removed `LoadFromPath(bool recursive = false, params string[] paths)` overload:
- Non-recursive calls should now use `LoadFromPath("path1", "path2", ...)`.
- Recursive calls should now use `LoadFromPathRecursive("path1", "path2", ...)`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- 添加ILogger用于日志记录替换GD.PushWarning - 修改GetAll方法返回ToArray()副本而非直接Values引用 - 分离路径加载方法为非递归和递归两个独立接口 - 新增LoadFromPath和LoadFromPathRecursive的重载方法 - 提取内部处理逻辑到ProcessEntry私有方法 - 优化目录遍历逻辑并改进错误处理机制 - 添加重复键检测和资源加载失败的日志记录
This was referenced Feb 28, 2026
This was referenced Mar 15, 2026
This was referenced May 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a generic repository abstraction and a Godot-specific resource repository supporting keyed access and bulk loading from filesystem paths.
New Features:
Enhancements: