Skip to content

[Enhancement] 下载整合包时自动配置图标 #4974

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.

Sign up for GitHub

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

Jump to bottom
Open
Calboot wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[Enhancement] 下载整合包时自动配置图标 #4974

Calboot wants to merge 10 commits into from

Conversation

@Calboot
Copy link
Contributor

@Calboot Calboot commented Dec 13, 2025 •
edited
Loading

Resolves #4774

@Calboot Calboot changed the title [Enhancement] Modpack icon [Enhancement] 下载整合包时自动配置图标 Dec 13, 2025
@Calboot Calboot marked this pull request as ready for review December 13, 2025 14:10
@3gf8jv4dv 3gf8jv4dv mentioned this pull request Dec 14, 2025
Closed
Glavo
Glavo reviewed Dec 24, 2025
View reviewed changes
Copy link
Member

@Glavo Glavo left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我认为不应该这样实现。是否要下载图标应该根据整合包具体情况而定，对于包含了图标的整合包就不应该下载图标，而不是这样一律下载图标。

这个功能之前我也实现了，只是因为还有些问题没修所以没提交，你看看是你在本 PR 里改，还是我来做吧。

而且我没看到清理图标临时文件的逻辑。

@Calboot
Copy link
Contributor Author

Calboot commented Dec 24, 2025

我认为不应该这样实现。是否要下载图标应该根据整合包具体情况而定，对于包含了图标的整合包就不应该下载图标，而不是这样一律下载图标。

这个功能之前我也实现了，只是因为还有些问题没修所以没提交，你看看是你在本 PR 里改，还是我来做吧。

而且我没看到清理图标临时文件的逻辑。

关于第一点，不可能在下载完成之前得知是否存在图标，所以我的方法是先解压资源包，如果检测到目标目录中有了就不复制过去。不过现在实现的确实不完善，你一说我才想起来。我明天改一下

第二点确实是我疏忽了

@Glavo
Copy link
Member

Glavo commented Dec 24, 2025

关于第一点，不可能在下载完成之前得知是否存在图标

所以应该先下载资源包，然后再按需下载图标。

@Calboot
Copy link
Contributor Author

Calboot commented Dec 25, 2025

所以应该先下载资源包，然后再按需下载图标。

图标也不大啊，下载下来不会有什么问题。
现在的问题在于整合包安装任务都在HMCLCore里面，导致我判断不了是不是已经存在图标文件了（因为图标文件的后缀名在FXUtils里定义）

所以好像还真得安装完再下载图标？

@Calboot
Copy link
Contributor Author

Calboot commented Dec 25, 2025 •
edited
Loading

我认为不应该这样实现。是否要下载图标应该根据整合包具体情况而定，对于包含了图标的整合包就不应该下载图标，而不是这样一律下载图标。

这个功能之前我也实现了，只是因为还有些问题没修所以没提交，你看看是你在本 PR 里改，还是我来做吧。

而且我没看到清理图标临时文件的逻辑。

做好了，而且现在不需要临时文件了

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

Reviewers

@Glavo Glavo Glavo left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Feature] 安装整合包时给实例添加图标

2 participants

@Calboot @Glavo