-
Notifications
You must be signed in to change notification settings - Fork 72
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
[+] Chuni Userbox with Assets #97
Conversation
Co-authored-by: split / May <[email protected]>
审阅者指南 by Sourcery这个拉取请求实现了 Chuni 用户框功能,包括显示用户的名牌和头像的能力。它引入了新的组件,利用 IndexedDB 存储 DDS 资源,并在设置菜单中添加了新的选项。 DDS 和用户框实现的类图classDiagram
class DDS {
-canvas2D: HTMLCanvasElement
-canvasGL: HTMLCanvasElement
-urlCache: Record<string, string>
-ctx: CanvasRenderingContext2D
-gl: WebGLRenderingContext
-ext: WebGLExtension
-shader: WebGLShader
-db: IDBDatabase
+load(buffer: Uint8Array)
+getBlob(inFormat?: string): Promise<Blob>
+fromBlob(input: Blob)
+loadFile(path: string): Promise<boolean>
+getFile(path: string, fallback?: string): Promise<string>
-compileShaders()
}
class ChuniPenguinComponent {
+chuniWear: number
+chuniHead: number
+chuniFace: number
+chuniSkin: number
+chuniItem: number
+chuniFront: number
+chuniBack: number
+classPassthrough: string
}
class ChuniUserplateComponent {
+chuniLevel: number
+chuniName: string
+chuniRating: number
+chuniNameplate: number
+chuniCharacter: number
+chuniTrophyName: string
}
DDS <-- ChuniPenguinComponent : uses
DDS <-- ChuniUserplateComponent : uses
文件级变更
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以:
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request implements the Chuni userbox feature, including the ability to display user's nameplates and avatars. It introduces new components, utilizes IndexedDB for storing DDS assets, and adds new options in the settings menu. Class diagram for DDS and Userbox implementationclassDiagram
class DDS {
-canvas2D: HTMLCanvasElement
-canvasGL: HTMLCanvasElement
-urlCache: Record<string, string>
-ctx: CanvasRenderingContext2D
-gl: WebGLRenderingContext
-ext: WebGLExtension
-shader: WebGLShader
-db: IDBDatabase
+load(buffer: Uint8Array)
+getBlob(inFormat?: string): Promise<Blob>
+fromBlob(input: Blob)
+loadFile(path: string): Promise<boolean>
+getFile(path: string, fallback?: string): Promise<string>
-compileShaders()
}
class ChuniPenguinComponent {
+chuniWear: number
+chuniHead: number
+chuniFace: number
+chuniSkin: number
+chuniItem: number
+chuniFront: number
+chuniBack: number
+classPassthrough: string
}
class ChuniUserplateComponent {
+chuniLevel: number
+chuniName: string
+chuniRating: number
+chuniNameplate: number
+chuniCharacter: number
+chuniTrophyName: string
}
DDS <-- ChuniPenguinComponent : uses
DDS <-- ChuniUserplateComponent : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嘿 @raymonable - 我已经审查了你的更改 - 以下是一些反馈:
整体评论:
- 在合并之前需要解决Safari兼容性问题,因为它是一个主要的浏览器平台
- 请为所有新字符串添加缺失的中文翻译
以下是我在审查期间查看的内容
- 🟢 一般问题:一切看起来都很好
- 🟢 安全性:一切看起来都很好
- 🟢 测试:一切看起来都很好
- 🟡 复杂性:发现1个问题
- 🟢 文档:一切看起来都很好
帮助我变得更有用!请在每条评论上点击 👍 或 👎,我将使用这些反馈来改进你的评论。
Original comment in English
Hey @raymonable - I've reviewed your changes - here's some feedback:
Overall Comments:
- Safari compatibility issues need to be resolved before merging as it's a major browser platform
- Please add the missing Chinese translations for all new strings
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -0,0 +1,314 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题(复杂性): 考虑将缓存逻辑提取到一个单独的类中,以简化DDS类并实现缓存行为的独立测试。
混合缓存逻辑和DDS加载/渲染会增加不必要的复杂性。考虑将缓存提取到一个单独的类:
class DDSCache {
private urlCache: Record<string, string> = {};
constructor(private db?: IDBDatabase) {}
async loadFromDB(path: string): Promise<Blob | null> {
if (!this.db) return null;
const transaction = this.db.transaction(["dds"], "readonly");
const request = transaction.objectStore("dds").get(path);
return new Promise((resolve) => {
request.onsuccess = () => resolve(request.result?.blob ?? null);
request.onerror = () => resolve(null);
});
}
getCachedUrl(path: string): string | undefined {
return this.urlCache[path];
}
cacheUrl(path: string, url: string) {
this.urlCache[path] = url;
}
}
然后通过注入缓存来简化DDS类:
export class DDS {
constructor(private cache: DDSCache) {
this.initializeWebGL();
}
async getFile(path: string, fallback?: string): Promise<string> {
const cached = this.cache.getCachedUrl(path);
if (cached) return cached;
const loaded = await this.loadAndRender(path) ||
(fallback && await this.loadAndRender(fallback));
if (!loaded) return "";
const url = URL.createObjectURL(await this.getBlob("image/png") ?? new Blob([]));
this.cache.cacheUrl(path, url);
return url;
}
private async loadAndRender(path: string): Promise<boolean> {
const blob = await this.cache.loadFromDB(path);
if (!blob) return false;
await this.fromBlob(blob);
return true;
}
}
这种分离:
- 使缓存逻辑可独立测试
- 简化错误处理路径
- 保持核心DDS功能专注于解析/渲染
- 使修改缓存行为更加容易
Original comment in English
issue (complexity): Consider extracting the caching logic into a separate class to simplify the DDS class and enable independent testing of the caching behavior.
The mixing of caching logic with DDS loading/rendering adds unnecessary complexity. Consider extracting the caching into a separate class:
class DDSCache {
private urlCache: Record<string, string> = {};
constructor(private db?: IDBDatabase) {}
async loadFromDB(path: string): Promise<Blob | null> {
if (!this.db) return null;
const transaction = this.db.transaction(["dds"], "readonly");
const request = transaction.objectStore("dds").get(path);
return new Promise((resolve) => {
request.onsuccess = () => resolve(request.result?.blob ?? null);
request.onerror = () => resolve(null);
});
}
getCachedUrl(path: string): string | undefined {
return this.urlCache[path];
}
cacheUrl(path: string, url: string) {
this.urlCache[path] = url;
}
}
Then simplify the DDS class by injecting the cache:
export class DDS {
constructor(private cache: DDSCache) {
this.initializeWebGL();
}
async getFile(path: string, fallback?: string): Promise<string> {
const cached = this.cache.getCachedUrl(path);
if (cached) return cached;
const loaded = await this.loadAndRender(path) ||
(fallback && await this.loadAndRender(fallback));
if (!loaded) return "";
const url = URL.createObjectURL(await this.getBlob("image/png") ?? new Blob([]));
this.cache.cacheUrl(path, url);
return url;
}
private async loadAndRender(path: string): Promise<boolean> {
const blob = await this.cache.loadFromDB(path);
if (!blob) return false;
await this.fromBlob(blob);
return true;
}
}
This separation:
- Makes caching logic independently testable
- Simplifies error handling paths
- Keeps core DDS functionality focused on parsing/rendering
- Makes it easier to modify caching behavior
Hi, question: are the postinstall scripts from |
It seems that the "input url" feature is removed in i18n files. Is this feature removed? I think we can keep both "input url" and "select game folder" and let the user to choose |
It looks like it was removed. Raymond's asleep, but since he seems to want to get this merged so quickly, I'll do my best to look at what was there previously and reimplement it myself... |
Should the Edit: I've had it encapsulate the AquaBox options as well; let me know if you wish for this change to be reverted |
Adds back the classic UserBox preview when AquaBox is disabled / unavailable
i figured not many people got use out of it being there, since there's no documentation on how to setup the files-- it isn't very useful for the average public server player |
I think they may only need an url published by some maybe official one in discord |
moved the DDS cache from dds.ts to ddsCache.ts and added caching for scaled images Co-authored-by: split / May <[email protected]>
Uhh I don't think it matters, like I did not run the postinstall and it worked fine. |
'userbox.new.activate_update': 'Update AquaBox (game files required)', | ||
'userbox.new.activate': 'Use AquaBox', | ||
'userbox.new.activate_desc': 'Enable displaying UserBoxes with their nameplate & avatar', | ||
'userbox.new.error.invalidFolder': 'The folder you selected is invalid. Ensure that your game\'s version is Lumi or newer and that the "A001" option pack is present.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean A000?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, my mistake
export async function userboxFileProcess(folder: FileSystemEntry, progressUpdate: (progress: number, progressString: string) => void): Promise<string | null> { | ||
if (!isDirectory(folder)) | ||
return t("userbox.new.error.invalidFolder") | ||
if (!(await validateDirectories(folder, "bin/option")) || !(await validateDirectories(folder, "data/A000"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the logic here might be incorrect. Currently it requires both of these paths to be present, but they should be optional if only one is present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean data/A000 is required 100% but bin/option might not be? but there's such an unlikely chance that someone using game data won't have an option
it's finally done
a few notes:
Summary by Sourcery
实现带有铭牌和头像自定义的Chuni用户框显示。
新功能:
测试:
Original summary in English
Summary by Sourcery
Implement Chuni userbox display with nameplate and avatar customization.
New Features:
Tests: