Conversation
|
Important Review skippedToo many files! This PR contains 215 files, which is 115 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (215)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| using System.Threading.Tasks; | ||
| using UnityEditor; | ||
| using UnityEngine; | ||
|
|
| @@ -0,0 +1,307 @@ | |||
| namespace GameFrameX.Runtime | |||
| { | |||
| /// <summary> | |||
AlianBlank
left a comment
There was a problem hiding this comment.
ShellUtility 没有考虑跨系统的问题
AlianBlank
left a comment
There was a problem hiding this comment.
Code Review 结论:Request Changes
重构主体方向没问题(Utility.Xxx 嵌套分部类 → 顶层 XxxUtility,meta GUID 全部保留,测试迁移完整),但存在编译级硬伤与大量超范围改动,建议先整改再合。
🔴 阻塞问题
1. 编译错误:GameFrameXCroppingHelper.cs 引用了不存在的类型
_ = typeof(GameFrameX.Runtime.marshalUtility); // 小写 m类声明是 MarshalUtility,C# 大小写敏感,此行必然 CS0246 —— 说明 PR 未经过编译验证。
2. PR 描述未填写
变更 / 验证 / 关联均为模板占位符。±12,000 行、215 文件的重构缺少:动机说明、命名规则(哪些 Helper 改名哪些保留)、编译与测试验证记录。
3. 裁剪保根列表回归
GameFrameXCroppingHelper 的 typeof 保根清单删除了 CameraHelper / UnityRendererHelper,但没有补上替代的 UnityEngineCameraExtension / UnityEngineRendererExtension。开启 managed stripping 的构建会把这两个扩展类裁掉。
🟠 高优先级
4. Scope creep:混入大量与"组织方式调整"无关的新功能,建议拆分独立 PR
| 内容 | 位置 |
|---|---|
| 276 行进程执行模块(接口/委托/事件) | Runtime/Utility/ShellUtility.cs |
| 反射改标题栏的 Editor 功能 | Editor/Misc/ProjectPathTitlebarModifier.cs |
| ToInt/ToLong/Print/WithColor/WithSize 等 49 行 | Runtime/Extension/Extension/StringExtensions.cs |
| GetAssetPath/GetMaterialTextures 等 | Editor/Extension/ObjectExtensions.cs |
| GetBytesSize/DeleteIfExists | Runtime/Utility/FileUtility.cs |
| 10 个新扩展方法 | Runtime/Extension/UnityEngine.GameObject/UnityEngine.GameObjectExtension.cs |
5. ShellUtility 放在 Runtime 程序集(安全 + 职责双重问题)
运行时程序集暴露 ExecuteCommand(command, args) 任意命令执行 API,会打进正式包与游戏构建。此类工具应归 Editor 程序集。另外:ShellArgs/ShellResult 用公有字段而非属性;行内 // 注释与项目 XML doc 风格不符;零测试。
6. ProjectPathTitlebarModifier.cs 多处问题
- 反射 Unity 非公开 API(
ApplicationTitleDescriptor、updateMainWindowTitle),Unity 升级随时失效;First(...)找不到类型直接抛异常 - 类名
ProjectTitleModifier≠ 文件名ProjectPathTitlebarModifier.cs - 无命名空间(项目其余代码都在
GameFrameX.*下) InitializeOnLoadMethod+_ = Task.Delay(2000)fire-and-forget hack
7. 破坏性 API 变更无迁移说明
Utility.Xxx 公开入口全部直接删除、无兼容层。下游工程(Unity 客户端工程就有 10+ 文件引用 Utility.Xxx、4 个引用旧 Helper 名)升级即编译失败。这属于 semver major 变更,需要迁移说明。GameObjectHelper.Create(Transform, string) 重载直接消失(变成 TransformExtension.CreateChild 扩展方法)也没有交代。
🟡 中优先级
8. UnityEngineGameObjectExtension 代码质量
Destroy(this GameObject)与UnityEngine.Object.Destroy极易混淆FindChildGamObjectByName保留拼写错误(Gam→Game),且同一逻辑在GameObjectUtility与GameObjectExtension两处重复实现SetLayer:if (gameObject.layer != layer)比较冗余;CachedTransforms.Clear()冗余(GetComponentsInChildren本身会清空列表);静态共享缓存列表有重入隐患;返回结果含自身与开头赋值重复- 扩展方法内
Debug.Assert(!ReferenceEquals(gameObject, null)):release 下 Assert 被剔除,变成远离调用点的 NRE
9. StringExtensions 的调试 API 进 Runtime
Print() 直接 Debug.Log 且默认绿色富文本 —— 框架已有 GameFrameworkLog 日志体系。ToInt/ToLong/ToFloat/ToDouble 与 ConverterUtility(941 行)职责重叠。
10. 命名不一致
HashUtility.MD5.cs/HashUtility.XXHash.cs(缩写全大写)vs 同族HashUtility.Sha1.cs/HashUtility.HMACSha256.cs;旧名Md5.cs/XxHash.cs反而符合 .NET 规范GameFrameworkText/Json/Compression(前缀式)与XxxUtility(后缀式)两套体系并存;EditorHelper/LitJsonHelper/TimerHelper等 Helper 名保留 —— 改名规则未说明HashUtility.cs是只有空{ }的 partial 壳文件,冗余
11. 大文件整文件重写,无法 review、blame 断裂
GameFrameworkLog.cs:rename 后 +2730 −2730,每行都变(纯移动应为 0/0),疑似编码/行尾/注释重排,patch 超限无法审查ZipUtility.cs+376 −377:逐行插英文 remarks 注释导致全文件 diffEditorHelper:移除 BOM + 清行尾空格混进功能改动
建议:纯移动的文件去掉混入的格式/注释改动;BOM/行尾统一单独提交。
12. GameFrameXCroppingHelper.cs 文件末尾丢失换行符(\ No newline at end of file)
13. EditorHelper 挪到 Editor 程序集且命名空间 GameFrameX.Runtime → GameFrameX.Editor,meta 换新 GUID(原 Utility.cs.meta 删除)。如有代码引用旧命名空间会断。
🟢 已核对、无需修改的点
- meta GUID 处理正确:内容大改的文件 meta 均为 rename(GUID 保留);
FileUtility的 GUID 取自Utility.File.cs.meta(FileHelper.cs.meta直接删)—— 纯静态类无序列化引用,可接受 - 测试迁移完整:
ObjectHelperTests(136 行)删除是去重(Swap 测试在UtilityObjectTests已保留);RemoveEmptyDirectory测试迁入UtilityDirectoryTests;RandomUtility测试净增 FileNameSuffix16 个常量 1:1 迁移无丢失;BaseComponent.cs388 行 diff 中 370 行是纯 API 引用替换- 未发现 C# 9+ 语法、无括号控制流等编码规范违规
建议整改顺序
- 修
marshalUtility编译错误 + 补 Camera/Renderer 裁剪保根(阻塞) - 补全 PR 描述:命名规则、迁移说明、验证记录
- 把 ShellUtility、ProjectPathTitlebarModifier、StringExtensions 等新功能拆出去单独 PR
- 纯移动的文件(
GameFrameworkLog.cs等)去掉混入的格式改动,恢复纯移动 diff
更正说明(Review 问题勘误)对前一条 review 做两处更正,其余问题经对 PR head 真实代码二次复核均属实。 1. 撤回「问题 1:marshalUtility 编译错误」——系误报,向作者致歉。 复核 PR head 实际代码, 2. 修正「问题 11」中关于全文件 diff 的归因:
附带观察(非问题): 本 PR 将 142 个文件的版权头从「MIT + Apache 2.0 双许可」统一为「仅 Apache 2.0」,与根 阻塞问题仅剩:补 |
变更
验证
关联
Fixes GFX-xxx
Closes #xxx