Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

重新设计剧情播放器数据格式和架构 #63

Open
mark9804 opened this issue Jun 29, 2023 · 14 comments
Open

重新设计剧情播放器数据格式和架构 #63

mark9804 opened this issue Jun 29, 2023 · 14 comments
Labels
enhancement New feature or request module: story-player 播放器的问题 refactor rfc 需要开发人员参与讨论

Comments

@mark9804
Copy link
Contributor

mark9804 commented Jun 29, 2023

目前由于架构和数据结构原因导致的问题:

  • 补充
  • 层级关系混乱,为了兼容tooltip导致多了一堆对应的event以及if
  • StoryUnit定义过于复杂,一堆不相关的东西都塞进去了,虽然有type做区分

当前数据结构和架构无法完成的需求:

  • variable(能做但要拉稀)
  • loop(能做但要 walkaround)

提案:

  • 补充
@diyigemt
Copy link
Contributor

diyigemt commented Jun 29, 2023

loop之前提到过实际上是可以实现的,加个customtag,读取到直接整个replace translate就行
问题是variable,这改动比较大,也不是不能实现对吧,但就变成在屎上拉稀屎的感觉了

提议: StoryUnit使用联合类型定义

type StoryUnitType = "title" | "place";
interface BaseStoryUnit<key extends StoryUnitType> {
  type: key;
}
type StoryUnitTitle = BaseStoryUnit<"title"> & { title: string };
type StoryUnitPlace = BaseStoryUnit<"place"> & { place: string };
type StoryUnit = StoryUnitTitle | StoryUnitPlace ;
function isTitleUnit(unit: StoryUnit): unit is StoryUnitTitle {
  return unit.type && unit.type === "title";
}
function doSomething(unit: StoryUnit) {
  if (isTitleUnit(unit)) {
    unit.title = "";
  }
}

@mark9804
Copy link
Contributor Author

我觉得现在问题比较大的点是事件驱动导致到处都是监听器,之前添加 tooltip 功能的时候就已经很难受了。上次修改summary 的 z-index 问题,拿不到 menu-hide 的状态,非得监听打开菜单的事件才能调整 z-index

我是觉得有一些状态是可以分享的

@mark9804 mark9804 added enhancement New feature or request refactor labels Jun 29, 2023
@notnotype
Copy link
Contributor

notnotype commented Jun 29, 2023

事件驱动是一个好设计,但是不要把 state 放到 event bus 里面来,我的建议是 event bus 只放控制播放器相关的指令(即事件只能由翻译层发送)。例如:

  • fullScreen 事件:播放器收到该事件后设置 state.fullScreen = true。这是剧情脚本里面控制的全屏
  • 如果是 UI 层点击全屏按钮则 UI 层直接修改 state.fullScreen = true
  • st 事件:即翻译层解析剧情脚本遇到了 st 命令,故发送该事件。想要展示一段 st 字幕。

这样的好处是可以将翻译层独立出来,只需要给翻译层提供一个发送事件的接口,播放器就可以运行了

@github-actions github-actions bot added the UI label Jun 29, 2023
@mark9804 mark9804 pinned this issue Jun 29, 2023
@mark9804
Copy link
Contributor Author

AnimationDone, VoideDone, L2dAnimationDone 之类的事件要怎么协调呢

@notnotype
Copy link
Contributor

notnotype commented Jun 29, 2023

AnimationDone, VoideDone, L2dAnimationDone、click、next 这些事件需要播放器反过来通知翻译层。可以再提供一个接口(给翻译层),作为播放器事件上报的接口。这样就变成了我们最熟悉的 C/S 架构

事件流向:

播放器                               翻译层
play(告诉翻译层开始播放剧情了)
                                     done(告诉播放器已经准备好了)
done(告诉翻译层给我下一条命令)
                                     st (请播放一条 st 字幕)
done
                                     sleep (请等待)
done
                                     playAnimation(播放动画)
done


这些事件可以是异步的,例如再翻译层可能有这种代码:

await st() // 等待st播放完毕
await playAnimation() // 等待动画播放完毕
let opt = await select('opt1', 'opt2')  // 等待用户选择

如果想要拓展翻译层的功能,使其能够感知播放器的状态,还可以这样:

player.on('fullScreen', async () => {})

这似乎有点过度设计了,但总之就是翻译层想遥控一样控制播放器。

@diyigemt
Copy link
Contributor

我的想法是保持现在的设计,只是单独整理播放器的代码,比如菜单的显示/隐藏作为store而不是事件,
上面提到的设计和现有的没有区别,只不过把播放器内部控制播放的功能转移到翻译器那边罢了,我是想让翻译器只做它名字本身的工作

@diyigemt diyigemt removed the UI label Jun 29, 2023
@mark9804
Copy link
Contributor Author

事件驱动设计保持不变,设置一个各个组件都能访问的状态 store,部分组件状态 (主要是 UI 层)由事件驱动改为状态驱动

是这样想的吗

@github-actions github-actions bot added the UI label Jun 29, 2023
@diyigemt
Copy link
Contributor

diyigemt commented Jun 29, 2023

对,以及层级问题或许可以用一下css的魔法,虽然没有发事件,但是我加个class,它兄弟或者儿子就能根据css的配置改index了

当然后期维护可能会出问题

@diyigemt diyigemt removed the UI label Jun 29, 2023
@mark9804
Copy link
Contributor Author

这有点黑了,层级一开始就设计好范围,每个层的范围只能限定在比如说10-30,这样做一个约束后期可能会好维护一些

另外目前的数据格式有什么问题可以开喷了

@diyigemt
Copy link
Contributor

但是层级问题是必须解决的,比如宝贝tooltip为了能被用户点到必须在最上层,但是如果它一直保持最上层,就会把本应在上层的东西覆盖掉,比如log,比如早就写好还没放上去的点击特效。
它必须得是动态的,光定个范围没用

@diyigemt
Copy link
Contributor

但是层级问题是必须解决的,比如宝贝tooltip为了能被用户点到必须在最上层,但是如果它一直保持最上层,就会把本应在上层的东西覆盖掉,比如log,比如早就写好还没放上去的点击特效。 它必须得是动态的,光定个范围没用

当然你想整成脏的也不是不行,每次点击计算范围是不是在tooltip里面,在就显示(

@notnotype
Copy link
Contributor

像这种需要保持再最上层的可以用 Transport

@diyigemt
Copy link
Contributor

宝贝 里面的东西都是position: absolute的

@diyigemt
Copy link
Contributor

而且tooltip为了实现响应式严重依赖它爹的css参数以及定位信息

宝贝 里面的东西都是position: absolute的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment