依据 docs/项目工程改进方案.md 实施(第五梯队 CI/协作除外)。
安全性
- Release 剥离 Log.v/d/i/w(-assumenosideeffects),保留 Log.e
- 全量频道/成员 dump、消息正文、发送者 UID 改为 BuildConfig.DEBUG 保护
(实测 release dex 中 "Channel: id=" / "Client: id=" / selfId= 等均为 0 命中)
构建产物体积
- .gitignore 补 *.a(保留 .opus/lib 四个 ABI 预编译库)、.opus/install、
app/libs/*.aar、dnn/torch
- git rm --cached 除名 AAR、.gradle、.opus/{build,install}、dnn/torch
跟踪体积 50MB → 17.4MB,工作区文件不受影响
跨语言契约(本方案核心)
- 新增 go/teamspeak/contract.go:字段名常量 + BridgeContractVersion 单一事实来源
- 新增契约测试:反射断言常量与 struct tag 一致;Go/Kotlin 共用 golden 样本
- 新增 BridgeContract.kt:启动校验 AAR 契约版本,不匹配则阻止连接
- 修正文档:channel_order 是前驱频道 ID(链表指针)而非排序权重,
按它数值排序会打乱频道树;ChannelInfo.order 在 Go 侧不存在
状态模型
- ConnectionState 增 Idle 取代 null 编码,connectionState 不再可空
- applyClients 改为按频道差分更新,避免全量刷新导致频道树整体重组
- 频道数据 freshness 统一由 Repository 维护,修复 ViewModel 与 Repository
两份时间戳不同步导致的重复刷新
UI
- 修复 collectAsState() 在参数位置调用导致的 isSelf 快照失效
- 消除 9 处 !! 断言(ChannelListScreen)
- 硬编码 24.5/14.5dp 与 depth*24 收敛到 UiTokens.Spacing,缩进加 4 层上限
测试
- Kotlin 测试 7 → 106;Go 契约测试新增 24 个用例
- 各覆盖 InputValidator、消息送达确认、僵尸会话过滤、频道差分、
频道顺序语义、错误分类
其他修复
- classifyError 提取为纯函数并补测试;修复 too many clones (id=521)
未识别导致英文原文直接暴露给用户
版本号提升至 1.0.13(versionCode 13)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
481 lines
29 KiB
Markdown
481 lines
29 KiB
Markdown
# TS Mobile 工程改进方案
|
||
|
||
> 编写依据:对 `android/`、`go/`、`.git` 的实测,以及 `CLAUDE.md`、`docs/` 现有文档的交叉核对。
|
||
> 立场:本文不复述通用 Android 最佳实践,只针对这个项目因**双层跨语言架构**而产生的真实风险。
|
||
> 所有结论均附实测证据,未验证的地方标注为「推测」。
|
||
|
||
---
|
||
|
||
## 一、我的核心判断
|
||
|
||
这个项目最大的风险不在 UI,也不在 Kotlin 代码质量,而在**Go 与 Kotlin 之间那道没有类型保护的边界**。
|
||
|
||
架构是:Go 协议层 → gomobile 编译成 `teamspeak.aar` → Kotlin 通过 JNI 调用。gomobile 有三条硬限制(`CLAUDE.md` 已记录):不能导出 `[]string`、不能导出 `[]*T`、不能导出 Go `error`。为了绕过它们,项目选择了**用 JSON 字符串传所有复杂数据**。
|
||
|
||
这个选择的代价是:
|
||
|
||
> **Go 侧改一个字段名,Kotlin 侧不会编译失败,只会在运行时静默拿到默认值。**
|
||
|
||
`Repository.kt:18` 的 `Json { ignoreUnknownKeys = true }` 让这个失效模式更隐蔽——字段名对不上时不报错,`ChannelInfo` 直接用默认值 `""` / `0` / `false` 填充。结果是频道列表看起来是空的,或者密码频道显示成公开频道,而日志里什么都没有。
|
||
|
||
而这条边界目前是**完全无保护**的:
|
||
|
||
| 保护手段 | 现状 |
|
||
| --- | --- |
|
||
| 编译期类型检查 | 无(JSON 字符串) |
|
||
| 契约文档 | 无(`docs/sdk-bridge-api.md` 存在但未核对与 `bridge.go` 是否同步) |
|
||
| 契约测试 | 无 |
|
||
| Schema 校验 | 无 |
|
||
| 版本标识 | 无(AAR 无版本号,`bridge.go` 无契约版本常量) |
|
||
|
||
叠加第二个事实:**重编 AAR 是手工两步流程,且产物入了 git**。`CLAUDE.md` 专门用一整节记录「忘记加 `-linkmode=external` 会 SIGSEGV」「忘记 `-Wl,--hash-style=both` 会 dlopen 失败」——这两个坑被写进文档,说明它们已经真实咬过人。而 `android/app/libs/teamspeak.aar`(23.13 MB)被 git 跟踪,意味着**你无法从提交历史判断某个 APK 用的是哪一版协议层**。
|
||
|
||
所以我的改进主线是一条:
|
||
|
||
> **先把跨语言契约固化成可验证的东西,再谈架构解耦,最后才谈 UI。**
|
||
|
||
顺序不能反。在一个契约不可靠的地基上拆 ViewModel、重构 Compose,只会把不确定性扩散到更多文件里。
|
||
|
||
---
|
||
|
||
## 二、实测到的问题分级
|
||
|
||
### 第一梯队 · 止血(半天内可完成,阻断其他改进)
|
||
|
||
#### 1.1 Release 包泄漏完整成员列表与 UID
|
||
|
||
`proguard-rules.pro` 全文只有 3 行:
|
||
|
||
```
|
||
-keep class teamspeak.** { *; }
|
||
-keep class com.tsmobile.app.TSBridge { *; }
|
||
```
|
||
|
||
没有任何日志剥离规则。而全项目有 **197 处 `Log.*` 调用**(实测统计),其中 `Repository.performInitialSync` 在 `Repository.kt:89-94` 逐条打印:
|
||
|
||
```kotlin
|
||
for (ch in channels) {
|
||
android.util.Log.d("Repository", "Channel: id=${ch.id}, name=${ch.name}, parentId=${ch.parentId}...")
|
||
}
|
||
for (cl in clients) {
|
||
android.util.Log.d("Repository", "Client: id=${cl.id}, nickname=${cl.nickname}, channelId=${cl.channelId}")
|
||
}
|
||
```
|
||
|
||
`applyClients` 还打印 UID 过滤细节。这些在 release 包里全部保留——任何人 `adb logcat` 就能拿到服务器完整的频道结构和所有成员的昵称 + UID。UID 在 TeamSpeak 里是永久身份标识,等同于账号 ID。
|
||
|
||
**改法**(`proguard-rules.pro` 追加):
|
||
|
||
```proguard
|
||
-assumenosideeffects class android.util.Log {
|
||
public static int v(...);
|
||
public static int d(...);
|
||
public static int i(...);
|
||
public static int w(...);
|
||
}
|
||
```
|
||
|
||
保留 `Log.e`。注意 `-assumenosideeffects` 只删调用,不删参数表达式的求值——`Repository.kt` 里这些日志的参数是字符串模板,含 `channels` 遍历,删除调用后循环体变空,R8 会一并消除。
|
||
|
||
同时应把 `performInitialSync` 里的全量 dump 改成 debug-only 或只打印数量。
|
||
|
||
**验收**:`assembleRelease` 后反编译 `classes.dex`,grep 不到 `"Channel: id="` 字面量。
|
||
**成本**:低(~1 小时)。**风险**:低。这是本方案性价比最高的一项。
|
||
|
||
#### 1.2 构建产物入库,`.git` 已 236 MB
|
||
|
||
实测被 git 跟踪的二进制:
|
||
|
||
| 文件 | 大小 | 问题 |
|
||
| --- | --- | --- |
|
||
| `android/app/libs/teamspeak.aar` | 23.13 MB | 每次重编 AAR 都产生新版本,历史永久膨胀 |
|
||
| `android/.gradle/8.11.1/executionHistory/executionHistory.bin` | 9.98 MB | Gradle 本地构建缓存,纯垃圾 |
|
||
| `go/teamspeak/.opus/{lib,install,build}/{abi}/libopus.a` | ~3.3 MB × 4 ABI × 3 份 | 同一份库存了三次 |
|
||
| `third_party/opus/dnn/torch/osce/resources/training_files.txt` | 5.32 MB | PyTorch DNN 训练资源,Android 构建完全用不到 |
|
||
|
||
`.gitignore` 里已经写了 `android/.gradle/` 和 `go/teamspeak/.opus/build/`,但**对已跟踪文件无效**——需要显式 `git rm --cached`。另外 `.gitignore` 有 `*.so`、`*.dylib`,**唯独漏了 `*.a`**,这是 libopus.a 全量入库的直接原因。
|
||
|
||
这里有一个需要你决策的权衡,我不替你定:
|
||
|
||
- `libopus.a` 入库是**有意的**(`CLAUDE.md` 写明「Pre-built static libraries live in ... for all four Android ABIs」,避免每次交叉编译 opus)。但 `lib/`、`install/`、`build/` 三份内容相同,**至少可以去掉两份**,省 ~26 MB。
|
||
- `teamspeak.aar` 入库则是纯负担。它应该由 `build.bat` 现场生成。如果你担心 clone 后没装 gomobile 无法构建,正确做法是 Git LFS 或把 AAR 挂到 Release 附件,而不是塞进 git 对象库。
|
||
|
||
**改法**:
|
||
```
|
||
# .gitignore 追加
|
||
*.a
|
||
```
|
||
```
|
||
git rm --cached android/.gradle -r
|
||
git rm --cached go/teamspeak/.opus/build -r
|
||
git rm --cached go/teamspeak/.opus/install -r
|
||
git rm --cached "android/app/src/main/cpp/third_party/opus/dnn" -r
|
||
```
|
||
|
||
`git rm --cached` 只从索引移除,**不删工作区文件**,构建不受影响。
|
||
|
||
至于清理已有的 236 MB 历史(`git filter-repo`),那是**不可逆的破坏性操作**,会重写所有 commit hash、要求所有克隆重新拉取。除非仓库已经大到影响日常操作,我不建议现在做。先止住增量即可。
|
||
|
||
**验收**:`git ls-files | ForEach-Object { ... }` 统计跟踪文件总体积,从当前 ~50 MB 降到 ~25 MB;新 clone 后 `build.bat` 能正常产出 APK。
|
||
**成本**:低(~1 小时)。**风险**:中——`third_party/opus/dnn` 是否真的不参与构建需要你确认一次(如果 `CMakeLists.txt` 引用了它,删掉会编译失败)。
|
||
|
||
#### 1.3 顺带确认:密钥管理是正确的
|
||
|
||
我特意查了这一项,结论是**没问题,不用改**:`keystore/`、`*.jks`、`*.keystore`、`android/local.properties` 都在 `.gitignore` 里,且 `git ls-files keystore/` 返回空——签名密钥从未入库。`build.gradle.kts:28-32` 通过 `providers.gradleProperty()` 读密码而非硬编码,也是对的。
|
||
|
||
写在这里是为了避免你在后续清理时误以为这里也有问题。
|
||
|
||
---
|
||
|
||
### 第二梯队 · 契约固化(本方案的核心,1~2 周)
|
||
|
||
#### 2.1 给 bridge 建立单一事实来源
|
||
|
||
**现状证据**:`TSBridge.kt:255-258` 有一个已经存在的契约缺口:
|
||
|
||
```kotlin
|
||
fun sendTextMessage(targetMode: Int, targetId: String, message: String): String {
|
||
// TODO: 重新编译 AAR 后使用 client?.sendTextMessage(targetMode, targetId, message)
|
||
// 当前 AAR 只有 sendChannelMessage,对应 targetMode=2
|
||
return when (targetMode) {
|
||
```
|
||
|
||
也就是说:**接收侧支持三种会话**(`ChatMessage.targetMode` 为 1=私聊 / 2=频道 / 3=服务器,`ServerViewModel.sendMessageNotification` 还会把标题区分为"频道消息"/"私聊消息",通知 deep link 还能跳转到指定会话),但**发送侧只实现了频道消息**。私聊能收到、能弹通知,但用户无法回复——收发能力不对称。
|
||
|
||
需要说明的是,这里**没有**静默失败:`else` 分支返回的是显式错误串 `"暂不支持该消息类型"`,按桥接约定(非空=错误)会被上层识别为失败。这个处理是正确的。
|
||
|
||
但它暴露了真正的问题:**这个能力缺口只存在于代码注释里**。`docs/` 的 27 份文档中没有任何一处记录「发送仅支持 mode=2」,`ChatMessage.targetMode` 的字段注释还写着 `1=私聊, 2=频道, 3=服务器`,读起来像三种都支持。下一个人(或下一个 AI 助手)在实现私聊 UI 时,会假设发送链路是通的。
|
||
|
||
> 待核实:该错误串在 UI 上的呈现路径我没有追到底——`ChatViewModel.sendMessage` 是否把它映射成 `MessageSendState.Failed` 并让用户看见,需要实测确认。如果只落日志不弹提示,用户会以为消息发出去了。
|
||
|
||
这类「文档与代码不同步」的缺口只能靠显式契约发现。建议:
|
||
|
||
1. 在 `go/teamspeak/` 下新建 `contract.go`,用一个常量集中声明契约版本与所有 JSON 字段名:
|
||
```go
|
||
const BridgeContractVersion = "1"
|
||
|
||
// 字段名常量,禁止在 marshal 处写字面量
|
||
const (
|
||
FieldChannelID = "id"
|
||
FieldChannelParentID = "parentId"
|
||
FieldChannelName = "name"
|
||
// ...
|
||
)
|
||
```
|
||
导出 `GetContractVersion()`,Kotlin 启动时校验,不匹配就明确报错而不是静默降级。
|
||
|
||
2. 在 `docs/` 下建 `bridge-contract.md`,逐个结构体列出:Go 类型 → JSON 字段 → Kotlin 数据类字段 → 是否可空 → 默认值语义。**每个字段标注双向支持状态**(如 `sendTextMessage: mode=2 已实现 / mode=1,3 未实现`)。
|
||
|
||
3. 把 `TSBridge.kt` 的 TODO 转成显式失败:未实现的 `targetMode` 返回明确错误串,让 `ChatViewModel` 能把消息标成 `FAILED` 而不是 `PENDING` 挂着。
|
||
|
||
**验收**:`bridge-contract.md` 覆盖 `bridge.go` 导出的全部结构体;启动时有契约版本校验;私聊发送失败会在 UI 上可见。
|
||
**成本**:中(2~4 天,主要是梳理)。**风险**:低(纯增量,不改行为)。
|
||
|
||
#### 2.2 给契约加测试
|
||
|
||
这是整个方案里我认为**最该投入**的一项。
|
||
|
||
现状:Kotlin 侧只有 2 个测试文件(`EmojiMapperTest.kt`、`MixedPcmFrameTest.kt`),Go 侧自有测试只有 `receive_audio_test.go`(343 行,其余 19 个 `_test.go` 属 vendored 的 teamspeak-go,不算你的资产)。
|
||
|
||
而核心逻辑规模是:`ServerViewModel.kt` ~1355 行、`bridge.go` 1542 行、`kotlin_api.go` 625 行、`ChannelViewModel.kt` 613 行、`Repository.kt` 325 行——**这些全部零测试**。
|
||
|
||
不需要追求覆盖率。只需要针对「跨边界 + 纯逻辑」这两类写测试,投入产出比最高:
|
||
|
||
**A. 契约往返测试(Go 侧)**——最高优先
|
||
```go
|
||
func TestChannelInfoJSON_ContractStable(t *testing.T) {
|
||
// 固定输入 → 断言 JSON 字段名与类型
|
||
// 任何人改字段名,这个测试立刻红
|
||
}
|
||
```
|
||
|
||
**B. 反序列化健壮性(Kotlin 侧)**
|
||
用 Go 侧产出的真实 JSON 样本作为 test resource,断言 `ChannelInfo` / `ClientInfo` / `ServerInfo` 能正确解析,且缺字段时的默认值是预期的。**特别是 `ignoreUnknownKeys = true` 掩盖的那些场景**。
|
||
|
||
**C. 纯逻辑单元测试**——这些现在就能写,无需 mock Android:
|
||
- `InputValidator`(4 个校验函数,含 `address:port` 解析与 1..65535 边界)
|
||
- `Repository.confirmMessageDelivery` / `markPendingMessagesFailed`(消息送达确认的匹配逻辑,含 `senderId=0` 降级路径)
|
||
- `ServerViewModel.classifyError`(6 类错误映射)
|
||
- `ChannelViewModel.mapMoveError`
|
||
- 僵尸会话过滤 `applyClients`(`Repository.kt:172-187`,同 UID 不同 clid 的过滤规则,这是重连正确性的关键)
|
||
|
||
`Repository` 是 `object` 单例,直接依赖 `TSBridge`,不好测。改法是抽出纯函数:把 `applyClients` 的过滤逻辑提成 `internal fun filterZombieSessions(clients: List<ClientInfo>, selfId: Int): List<ClientInfo>`,单独测。这类重构成本极低但立刻可测。
|
||
|
||
**验收**:CI 上跑通;改动 `bridge.go` 任一字段名会导致测试失败。
|
||
**成本**:中(3~5 天)。**风险**:低。
|
||
|
||
#### 2.3 拆掉 ViewModel 之间的手工交叉引用
|
||
|
||
`NavGraph.kt:66-71`:
|
||
|
||
```kotlin
|
||
LaunchedEffect(Unit) {
|
||
serverViewModel.channelViewModel = channelViewModel
|
||
serverViewModel.chatViewModel = chatViewModel
|
||
serverViewModel.voiceViewModel = voiceViewModel
|
||
channelViewModel.chatViewModel = chatViewModel
|
||
}
|
||
```
|
||
|
||
配合 `ServerViewModel.kt:85-89` 的三个 `@Volatile var ...ViewModel?`。
|
||
|
||
这是**用可变引用做依赖注入**,问题有三:
|
||
1. 赋值发生在 `LaunchedEffect` 里,即首帧之后。在此之前的任何 bridge 回调都会走 `?:` 兜底分支——`ServerViewModel.kt:1206/1213/1220` 三处 `?: viewModelScope.launch { Repository.refreshClientList() }` 就是这个兜底。
|
||
2. ViewModel 生命周期由 `viewModel()` 在 NavGraph 作用域创建,理论上同生共死,但这个不变量没有任何机制保证,靠注释维系。
|
||
3. 状态所有权模糊:`Repository` 是真相源,但 `ChannelViewModel` 也持有 `currentChannelId`、`channels`、`clients` 的镜像 StateFlow,两边都要同步。
|
||
|
||
**根因**是 Go 回调只有一个入口(`createBridgeCallbacks()`,`ServerViewModel.kt:993` 起,一个函数吞掉所有事件再手工分发)。
|
||
|
||
**改法**(渐进,不需要引入 DI 框架):
|
||
1. 新建 `BridgeEventBus`(单例 object,内部 `MutableSharedFlow<BridgeEvent>`)。
|
||
2. `TSBridge.Callbacks` 的实现从 `ServerViewModel` 移到一个独立的 `BridgeDispatcher`(由 `App` 或 `ConnectionService` 持有,生命周期与应用一致,而非与 ViewModel 一致)。
|
||
3. `BridgeDispatcher` 只做一件事:把 JNI 回调转成 sealed class 事件投递到 bus。
|
||
4. 各 ViewModel 自己 `collect` 关心的事件,不再互相持有引用。
|
||
|
||
这一步同时解决了一个潜在 bug:**当前 ViewModel 被清除后(如配置变更、进程重建),bridge 回调就没人接了**。把 dispatcher 挂到应用级生命周期更稳。
|
||
|
||
**验收**:`ServerViewModel` 中不再出现其他 ViewModel 的类型引用;grep `@Volatile var.*ViewModel` 无结果。
|
||
**成本**:高(4~7 天)。**风险**:中——涉及所有回调路径,**必须在 2.2 的测试到位之后做**。这是梯队顺序不能反的具体原因。
|
||
|
||
#### 2.4 拆分 ServerViewModel
|
||
|
||
~1355 行,实测承担 **9 类职责**:连接生命周期、重连状态机(含 `reconnectLock` / `reconnectAttempt` / `reconnectJob` / `lastConnectParams` / `lastChannelId` 五个并发字段)、网络监控、系统通知构建与发送(`sendPokeSystemNotification` / `sendMessageNotification` / `clearNotifications` / `triggerVibration`)、Poke 气泡、主题切换、应用更新检测(`checkForUpdate` / `manualCheckUpdate` / `closeUpdateDialog` / `openReleaseUrl` / `dismissUpdate`)、表单状态、bridge 回调分发、错误分类。
|
||
|
||
其中**更新检测和主题切换与"服务器"毫无关系**,纯粹是历史堆积。
|
||
|
||
**改法**(按可独立性排序,前三项可立刻做,互不阻塞):
|
||
| 抽出 | 目标 | 依赖 |
|
||
| --- | --- | --- |
|
||
| 更新检测(5 个方法 + `UpdateInfo` + `updatePreferences`) | `UpdateViewModel` | 无,完全独立 |
|
||
| 主题(`toggleTheme` + `themeMode` + `themePreferences`) | `SettingsViewModel` 或 `CompositionLocal` | 无 |
|
||
| 通知构建 | `NotificationHelper`(非 ViewModel,纯工具类) | 需 `Application` context |
|
||
| 重连状态机 | `ReconnectController` | 依赖 2.3 完成 |
|
||
| bridge 回调分发 | `BridgeDispatcher` | 即 2.3 |
|
||
|
||
拆完 `ServerViewModel` 应只剩连接生命周期 + 表单状态,预计 ~350 行。
|
||
|
||
**成本**:中(前三项 2~3 天,后两项依赖 2.3)。**风险**:低~中。
|
||
|
||
---
|
||
|
||
### 第三梯队 · 状态模型收敛(1 周)
|
||
|
||
#### 3.1 七个状态机,两处定义,语义重叠
|
||
|
||
实测项目里的状态机:
|
||
|
||
| 状态机 | 位置 | 取值 |
|
||
| --- | --- | --- |
|
||
| `ConnectionState` | `Models.kt:160` | Connected / Disconnecting / Reconnecting(attempt,max,reason) / WaitingForNetwork(since) / Disconnected(reason,wasKicked) |
|
||
| `ConnectState` | `ServerViewModel.kt:34` | IDLE / CONNECTING / SUCCESS / FAILED / TIMEOUT |
|
||
| `SyncState` | `ChannelViewModel.kt:20` | Unsynced / Syncing / Synchronized / SyncFailed |
|
||
| `ChannelSwitchState` | `Models.kt` | Requesting / Failed / ... |
|
||
| `VoiceState` | `Models.kt:196` | Idle / Transmitting / Blocked(reason) |
|
||
| `MessageSendState` | `Models.kt:138` | Idle / Sending / Failed |
|
||
| `MessageDeliveryState` | `Models.kt:72` | PENDING / SENT / FAILED |
|
||
|
||
`ConnectState` 和 `ConnectionState` 是**同一件事的两套表达**:一个是 enum 一个是 sealed class,一个管按钮外观一个管导航。`NavGraph.kt:103` 用 `connectionState` 驱动导航,`ServerConfigScreen` 用 `state.connectState` 驱动按钮,两者的转换点分散在 `doConnect()` 里。这就是「Connected 但 UI 还停在 CONNECTING」这类 bug 的温床。
|
||
|
||
`connectionState` 还用了 `StateFlow<ConnectionState?>`,`null` 表示"未进入频道列表页"(`ServerViewModel.kt:101-103` 注释)——**用 null 编码一个业务状态**,每个消费点都要先判空。
|
||
|
||
**改法**:
|
||
1. 删除 `ConnectState`,`ServerScreenState` 直接引用 `ConnectionState` 的派生值。按钮外观由 `when(connectionState)` 计算,不作为独立状态存储。
|
||
2. `ConnectionState` 增加 `object Idle`(或 `data class Initial`)替代 `null`。
|
||
3. 把 `SyncState` 合并进 `ConnectionState`——「已连接但数据未就绪」本来就是一个连接阶段,不该是另一个维度。建议:
|
||
```kotlin
|
||
sealed class ConnectionState {
|
||
object Idle
|
||
data class Connecting(val target: String)
|
||
data class Syncing(val progress: SyncPhase) // ← 原 SyncState
|
||
object Connected // 数据已就绪
|
||
data class Reconnecting(val attempt: Int, val maxAttempts: Int, val reason: String)
|
||
data class WaitingForNetwork(val since: Long)
|
||
object Disconnecting
|
||
data class Disconnected(val reason: String, val wasKicked: Boolean)
|
||
}
|
||
```
|
||
这样「不存在 Connected but no channel」从一个**需要靠 UI 层自律维持的约定**,变成**类型系统保证的不变量**——只有 `Syncing` 完成后才可能是 `Connected`。
|
||
|
||
这一条直接消除了现有设计里最容易出错的一块,而且让 `NavGraph` 的导航逻辑从「监听多个状态 + 判空」简化为「单一 `when`」。
|
||
|
||
**验收**:grep 不到 `ConnectState`;`connectionState` 类型不再是可空。
|
||
**成本**:中(3~5 天)。**风险**:中——影响导航与所有 UI 消费点,需 2.2 的测试兜底。
|
||
|
||
#### 3.2 频道切换的乐观更新语义
|
||
|
||
`CLAUDE.md` 记录了两个 SDK 硬限制:
|
||
- 无 channel create/update/delete 事件,频道列表靠「5 分钟陈旧或切换前刷新」
|
||
- `notifycliententerview` 的 ChannelID 不可靠,client enter/leave 触发**全量 clientlist 刷新**
|
||
|
||
这两条对 UI 有直接影响,但代码里没有体现为显式设计:
|
||
- 全量刷新意味着 `_channelClients` 整个 Map 被替换(`Repository.kt:185`),成员列表会整体重组。高频进出场景(公共服务器)下这是持续的重组压力。
|
||
- 频道树数据可能滞后 5 分钟,UI 却没有任何「数据可能不是最新」的表达。
|
||
|
||
**改法**:
|
||
1. `applyClients` 改为**差分更新**:对比新旧列表,只对变化的 channelId 更新 `_channelClients` 的对应条目,而不是 `groupBy` 全量替换。
|
||
2. 给频道数据加 `fetchedAt` 时间戳,超过阈值时在 UI 上标注(如频道页下拉刷新 + "数据更新于 X 分钟前")。
|
||
3. 频道切换必须是**悲观**的:`ChannelSwitchState.Requesting` 期间禁用其他频道点击,等服务端确认或超时失败后回滚。现有 `ChannelConfirmDialog` + `switchState is ChannelSwitchState.Failed` 已经是这个思路,需要确认超时路径完整(**推测**:未见明确超时,若服务端不响应可能永久停在 Requesting——需实测)。
|
||
|
||
**成本**:中。**风险**:中(差分更新容易引入不一致,必须有测试)。
|
||
|
||
---
|
||
|
||
### 第四梯队 · UI 层(1~2 周,可与第三梯队并行)
|
||
|
||
#### 4.1 一个真实的 Compose 误用
|
||
|
||
`ChannelListScreen.kt:276`:
|
||
|
||
```kotlin
|
||
ClientActionMenu(
|
||
client = selectedClient!!,
|
||
isSelf = selectedClient!!.id == channelViewModel.selfClientId.collectAsState().value,
|
||
```
|
||
|
||
`collectAsState()` 在**参数表达式位置**调用。虽然 `ClientActionMenu` 是 `@Composable`,参数在 composable 作用域内求值,所以订阅能生效——但 `.value` 立即取值再传入,导致 `isSelf` 是**一次性快照**,`selfClientId` 变化时 `ClientActionMenu` 不会重组(它收到的是 Boolean,不是 State)。
|
||
|
||
同文件内 `selectedClient!!` 出现 9 次非空断言,配合 `var selectedClient by remember { mutableStateOf<ClientInfo?>(null) }`。这是典型的「用 !! 绕过可空性」,一旦 `onDismiss` 与回调时序交错就会崩。
|
||
|
||
**改法**:
|
||
```kotlin
|
||
selectedClient?.let { client ->
|
||
val selfId by channelViewModel.selfClientId.collectAsState()
|
||
ClientActionMenu(client = client, isSelf = client.id == selfId, ...)
|
||
}
|
||
```
|
||
用 `?.let` 消掉全部 `!!`,`collectAsState()` 提到函数体顶部。
|
||
|
||
**成本**:低(~1 小时)。**风险**:低。建议优先做,这是可复现的正确性问题。
|
||
|
||
#### 4.2 拆分 838 行的 ChannelListScreen
|
||
|
||
该文件含 12 个 `@Composable`/私有函数,单函数最长约 200 行(`ChannelListScreen` 本体,含 4 个弹窗的编排)。13 个 `collectAsState()` 集中在顶部——**任何一个状态变化都会重组整个屏幕**,包括频道树。
|
||
|
||
**改法**:
|
||
1. 把 4 个弹窗(`ChannelDetailCard` / `ClientActionMenu` / `PokeDialog` / `ChannelConfirmDialog`)提成一个 `ChannelListDialogs` composable,状态用参数传入。
|
||
2. `ChannelTreeContent` 已经独立(`:464`),但它自己 `collectAsState` 了 `channels` / `clients` / `expandedChannelIds` / `selfClientId` 四个流——保持这样是对的,不要提到父级。
|
||
3. `ChannelRow` / `ClientRow` 加 `key`(LazyColumn 已用 `channelTreeNodeItems` 扩展,需确认 key 稳定性)。
|
||
4. 高频变化的 Speaking 状态应下推到 `ClientRow` 内部订阅,不经过父级。
|
||
|
||
**成本**:中(2~3 天)。**风险**:低。
|
||
|
||
#### 4.3 硬编码尺寸
|
||
|
||
`ChannelListScreen.kt:48-55`:
|
||
|
||
```kotlin
|
||
private val TREE_LEFT_INDENT = 24.5.dp
|
||
private val TREE_RIGHT_INDENT = 24.5.dp
|
||
private val FOOTER_LEFT_INDENT = 14.5.dp
|
||
private val FOOTER_RIGHT_INDENT = 14.5.dp
|
||
```
|
||
|
||
以及 `:569` 的 `depth * 24).dp` 内联计算、`:565` / `:645` 的 `padding(vertical = 12.dp)`。
|
||
|
||
`UiTokens.Spacing` 已经有完整档位(None/ExtraSmall/Small/Medium/Large/ExtraLarge/Huge),这些 24.5 / 14.5 是绕开 token 的局部修正值。**24.5dp 这种半像素值在非整数密度屏幕(如 2.75x)上会产生子像素对齐问题**,导致缩进线模糊。
|
||
|
||
另外 `depth * 24` 无上限——TeamSpeak 频道可深层嵌套,5 层以上在窄屏必然溢出。需要定 `maxIndentDepth`(建议 4)并对超限部分做截断或横向滚动。
|
||
|
||
**改法**:全部替换为 `UiTokens.Spacing.*`,缩进改为 `UiTokens.Spacing.Large * min(depth, 4)`。
|
||
|
||
**成本**:低。**风险**:低。
|
||
|
||
#### 4.4 EmojiMapper.kt 1989 行
|
||
|
||
项目最大的 Kotlin 文件,是手写 emoji 映射表。这类**纯数据**不该占编译单元:它拖慢增量编译(改一行重编 1989 行),且无法被非 Kotlin 侧复用。
|
||
|
||
**改法**:移到 `assets/emoji-map.json`,启动时懒加载;或用 KSP 生成。有 `EmojiMapperTest.kt` 兜底,迁移风险可控。
|
||
|
||
**成本**:低~中。**风险**:低。
|
||
|
||
---
|
||
|
||
### 第五梯队 · 门禁与协作(持续)
|
||
|
||
#### 5.1 没有 CI
|
||
|
||
实测:仓库内无 `.github/workflows/`(`third_party/opus/.gitlab-ci.yml` 是上游 opus 自带的,与本项目无关),无 detekt/ktlint 配置,无 `lint.xml`。
|
||
|
||
构建全靠本地 `build.bat` / `build.sh`,而这两个脚本封装的正是 `CLAUDE.md` 里记录过会踩坑的手工两步流程。**没有 CI 意味着那两个 SIGSEGV / DT_HASH 坑只能靠人记住。**
|
||
|
||
**改法**:加一个最小 GitHub Actions(或任何你用的 CI):
|
||
```yaml
|
||
jobs:
|
||
go: # cd go && go vet ./teamspeak/ && go test ./teamspeak/
|
||
kotlin: # cd android && ./gradlew lint testDebug assembleDebug
|
||
aar: # 用固定 flags 重编 AAR,校验产物可被 dlopen(这条最关键)
|
||
```
|
||
第三项最有价值:把 `build.bat` 里的 `-ldflags` 写成 CI 里的唯一真源,让「忘记加 flag」变成不可能。
|
||
|
||
同时加 `detekt` + `ktlint`(Gradle 插件,配置成本低),并把 `hardcoded dp/sp` 之类规则打开——这能从机制上防止 4.3 的问题复发,比写进规范文档有效得多。
|
||
|
||
**成本**:中(1~2 天搭好)。**风险**:低。
|
||
|
||
#### 5.2 文档体系需要收口
|
||
|
||
`docs/` 下现有:`UI架构设计.md`(3 页 3 卡)、`状态控制设计.md`、`sdk-bridge-api.md`、`sdk文档-go.md`、`TS Mobile UI UX 重构设计规范.md`、`流程/00~09`(10 份)、`implementation/00~12`(13 份)——**27 份文档,总计约 400 KB**。
|
||
|
||
问题不是文档少,是**没有权威源声明**。`UI架构设计.md` 描述的三页三卡架构与 `TS Mobile UI UX 重构设计规范.md` 的 Server Space 架构直接冲突,而代码实现的是前者。一个新加入的人(或一个 AI 助手)无法判断该信哪份。
|
||
|
||
**改法**:在 `docs/README.md` 建一个索引表,每份文档标注:状态(权威 / 历史 / 草案)、覆盖范围、最后核对日期、与代码的一致性。冲突的明确标注「已被 X 取代」。
|
||
|
||
`CLAUDE.md` 是目前**唯一与代码高度一致**的文档(Known SDK Limitations、Thread Safety、Critical Build Details 三节我逐条核对过,全部属实)。建议把它提升为文档体系的入口,其余文档从它链接出去。
|
||
|
||
**成本**:低(半天)。**风险**:无。
|
||
|
||
---
|
||
|
||
## 三、明确不建议做的事
|
||
|
||
这部分和上面同样重要——避免你把力气花在错的地方。
|
||
|
||
1. **不要现在引入 Hilt / Koin**。项目是单人维护规模(`versionCode 12`、`versionName 1.0.12`),ViewModel 交叉引用的问题用 2.3 的事件总线就能解决,引入 DI 框架会带来 KSP 编译开销和学习成本,收益不成比例。
|
||
|
||
2. **不要重写 Compose UI**。`ui/components/` 下 20 个组件的粒度基本合理(多数 100~300 行),真正失控的只有 `ChannelListScreen`。局部拆分即可。
|
||
|
||
3. **不要动 Go 音频管线**。`receive_audio.go` 是全项目唯一有像样测试的模块(343 行测试对 281 行实现),且 `CLAUDE.md` 记录了 jitter buffer / Opus 解码 / 立体声混音的完整设计与「Speaking 检测由 Go 侧负责,Kotlin 不得从 PCM 推断」的所有权划分。这块是项目质量最高的部分,改它风险最大收益最小。
|
||
|
||
4. **不要清理 git 历史**(除非仓库已影响日常操作)。见 1.2,先止住增量。
|
||
|
||
5. **不要在契约固化之前做 2.3 / 3.1**。这两项都会大范围移动回调与状态流转路径。没有测试网兜着做,等于在无保护状态下重写核心链路。**这是我把它排在第二梯队而非第一梯队的唯一原因**——不是它不重要,是它有前置依赖。
|
||
|
||
6. **不要追求测试覆盖率指标**。这个项目该测的是「跨边界的契约」和「纯逻辑的状态转换」,UI 层用 Compose Preview + 手工验证足够。定一个 60% 覆盖率目标会让你去测 getter/setter。
|
||
|
||
---
|
||
|
||
## 四、落地顺序
|
||
|
||
按依赖关系排,不是按重要性排:
|
||
|
||
```
|
||
第 1 步(半天) 1.1 Release 剥离日志 ← 隐私问题,独立,立刻做
|
||
第 2 步(半天) 1.2 构建产物出库 ← 独立,立刻做
|
||
第 3 步(1 小时) 4.1 修 collectAsState 误用 ← 独立,正确性问题
|
||
第 4 步(1 天) 5.1 CI 最小门禁 ← 独立,且为后续所有步骤提供安全网
|
||
─────────────────────────────────────────
|
||
第 5 步(2~4 天) 2.1 契约文档 + 版本常量 ← 后续步骤的地基
|
||
第 6 步(3~5 天) 2.2 契约测试 + 纯逻辑测试 ← 2.3 / 3.1 的前置条件
|
||
─────────────────────────────────────────
|
||
第 7 步(2~3 天) 2.4 拆出 Update / Theme / Notification ← 独立,可提前做
|
||
第 8 步(4~7 天) 2.3 事件总线,消除 ViewModel 交叉引用
|
||
第 9 步(3~5 天) 3.1 状态模型收敛
|
||
─────────────────────────────────────────
|
||
第 10 步(并行) 4.2 / 4.3 / 4.4 UI 层清理
|
||
第 11 步(并行) 3.2 差分更新 + 频道数据时效
|
||
第 12 步(半天) 5.2 文档收口
|
||
```
|
||
|
||
第 1~4 步互不依赖,可以在一天内全部完成,且立刻消除一个隐私泄漏、一个正确性 bug,并建立安全网。**建议就从这四步开始。**
|
||
|
||
第 5~6 步是投资,做完之后第 8~9 步的风险从「高」降到「中」。
|
||
|
||
---
|
||
|
||
## 五、如果只做一件事
|
||
|
||
如果时间只够做一项,我会选 **2.2 的契约往返测试**。
|
||
|
||
理由:这个项目所有的结构性风险——JSON 字段静默失效、收发能力不对称却无处记录、AAR 与源码不同步、重编 flag 遗漏——本质上都是**同一个问题的不同表现:跨语言边界上没有可执行的验证**。而契约测试是唯一能用最小成本(一个 Go 测试文件 + 几个 Kotlin 测试)把这个边界钉住的手段。
|
||
|
||
其他所有改进都是在已知问题上打补丁;只有这一项能防止**你还不知道的问题**继续产生。
|