refactor(modpack): split modpack module and extract curseforge api#127
Merged
Conversation
|
@Wsrsq is attempting to deploy a commit to the retrofor Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Reviewer's GuideRefactors the modpack subsystem into a modular, testable architecture with a dedicated CurseForge HTTP client, pluggable parsers/resolvers/extractors, and explicit support for Modrinth, CurseForge, and MultiMC formats, plus end‑to‑end tests for import and override extraction. Sequence diagram for ModpackApi import flow with CurseForge resolutionsequenceDiagram
actor User
participant ModpackApi
participant ZipModpackParser
participant Archive
participant Formats as formats_parse
participant ModrinthParser as modrinth_parse
participant CurseForgeParser as curseforge_parse
participant MultiMCParser as multimc_parse
participant ResolverChain
participant CurseForgeFileResolver
participant CurseForgeApi
participant CurseForgeHTTP as CurseForge_HTTP_API
User->>ModpackApi: import(path)
ModpackApi->>ZipModpackParser: parse(path)
ZipModpackParser->>Archive: open(path)
activate Archive
Archive-->>ZipModpackParser: Archive
deactivate Archive
ZipModpackParser->>Formats: parse(archive)
alt Modrinth pack
Formats->>ModrinthParser: parse(archive)
ModrinthParser-->>Formats: ParsedModpack(modpack_type = modrinth)
else CurseForge pack
Formats->>CurseForgeParser: parse(archive)
CurseForgeParser-->>Formats: ParsedModpack(modpack_type = curseforge)
else MultiMC pack
Formats->>MultiMCParser: parse(archive)
MultiMCParser-->>Formats: ParsedModpack(modpack_type = multimc)
end
Formats-->>ZipModpackParser: ParsedModpack
ZipModpackParser-->>ModpackApi: ParsedModpack
ModpackApi->>ResolverChain: resolve(modpack)
activate ResolverChain
ResolverChain->>CurseForgeFileResolver: resolve(modpack)
alt modpack_type is curseforge
CurseForgeFileResolver->>CurseForgeFileResolver: resolve_files(files)
CurseForgeFileResolver->>CurseForgeApi: get_files(file_ids)
activate CurseForgeApi
CurseForgeApi->>CurseForgeHTTP: POST /v1/mods/files
CurseForgeHTTP-->>CurseForgeApi: files JSON
CurseForgeApi-->>CurseForgeFileResolver: CurseForgeGetFilesResponse
deactivate CurseForgeApi
CurseForgeFileResolver->>CurseForgeApi: get_mods(mod_ids)
activate CurseForgeApi
CurseForgeApi->>CurseForgeHTTP: POST /v1/mods
CurseForgeHTTP-->>CurseForgeApi: mods JSON
CurseForgeApi-->>CurseForgeFileResolver: CurseForgeGetModsResponse
deactivate CurseForgeApi
CurseForgeFileResolver->>CurseForgeFileResolver: map_curseforge_file(file, class_id)
CurseForgeFileResolver-->>ResolverChain: ParsedModpack(with concrete ModpackFile entries)
else other modpack_type
CurseForgeFileResolver-->>ResolverChain: ParsedModpack(unchanged)
end
ResolverChain-->>ModpackApi: ParsedModpack
deactivate ResolverChain
ModpackApi-->>User: ParsedModpack
Sequence diagram for ModpackApi override extraction flowsequenceDiagram
actor User
participant ModpackApi
participant ZipOverrideExtractor
participant Archive
participant FS as Filesystem
participant Progress as ProgressReporter
User->>ModpackApi: extract_overrides(path, game_dir, override_prefixes, on_progress)
ModpackApi->>Progress: ProgressReporter from on_progress
ModpackApi->>ZipOverrideExtractor: extract(path, game_dir, override_prefixes, reporter)
ZipOverrideExtractor->>Archive: open(path)
activate Archive
Archive-->>ZipOverrideExtractor: Archive
ZipOverrideExtractor->>Archive: list_names()
Archive-->>ZipOverrideExtractor: all_names
ZipOverrideExtractor->>ZipOverrideExtractor: select existing override_prefixes
ZipOverrideExtractor->>ZipOverrideExtractor: compute total extractable entries
loop for each entry in archive
ZipOverrideExtractor->>Archive: by_index(index)
Archive-->>ZipOverrideExtractor: entry
ZipOverrideExtractor->>ZipOverrideExtractor: strip(prefix, name) to relative
alt entry is in overrides
ZipOverrideExtractor->>Filesystem: create_dir_all(parent)
alt entry is directory
Filesystem-->>ZipOverrideExtractor: ok
else entry is file
ZipOverrideExtractor->>Filesystem: File::create(outpath)
Filesystem-->>ZipOverrideExtractor: file handle
ZipOverrideExtractor->>Filesystem: copy(entry, file)
Filesystem-->>ZipOverrideExtractor: bytes_written
end
ZipOverrideExtractor->>Progress: report(current, total, relative)
else not an override entry
ZipOverrideExtractor-->>ZipOverrideExtractor: skip
end
end
deactivate Archive
ZipOverrideExtractor-->>ModpackApi: ok
ModpackApi-->>User: ok
Class diagram for the refactored modpack subsystemclassDiagram
class ModpackApi {
+ModpackApi new()
+ModpackApi with_components(parser: ModpackParser, resolver: ModpackFileResolver, extractor: OverrideExtractor)
+ModpackInfo detect(path: Path)
+ParsedModpack import(path: Path)
+void extract_overrides(path: Path, game_dir: Path, override_prefixes: Vec~String~, on_progress: FnMut_usize_usize_str_)
-parser: ModpackParser
-resolver: ModpackFileResolver
-extractor: OverrideExtractor
}
class ModpackParser {
<<trait>>
+ParsedModpack parse(path: Path)
}
class ZipModpackParser {
+ZipModpackParser new()
+ParsedModpack parse(path: Path)
}
class ModpackFileResolver {
<<trait>>
+ParsedModpack resolve(modpack: ParsedModpack)
}
class ResolverChain {
+ResolverChain new(resolvers: Vec~ModpackFileResolver~)
+void push(resolver: ModpackFileResolver)
+ParsedModpack resolve(modpack: ParsedModpack)
-resolvers: Vec~ModpackFileResolver~
}
class CurseForgeFileResolver {
+CurseForgeFileResolver new(api: CurseForgeApi)
+ParsedModpack resolve(modpack: ParsedModpack)
-Vec~ModpackFile~ resolve_files(files: Vec~ModpackFile~)
-HashMap~u64_u64~ class_ids(mod_ids: Vec~u64~)
-api: CurseForgeApi
}
class OverrideExtractor {
<<trait>>
+void extract(path: Path, game_dir: Path, override_prefixes: Vec~String~, reporter: ProgressReporter)
}
class ZipOverrideExtractor {
+ZipOverrideExtractor new()
+void extract(path: Path, game_dir: Path, override_prefixes: Vec~String~, reporter: ProgressReporter)
}
class ProgressReporter {
<<trait>>
+void report(current: usize, total: usize, name: str)
}
class CurseForgeApi {
+CurseForgeApi new(client: Client)
+CurseForgeGetFilesResponse get_files(request: CurseForgeGetModFilesRequestBody)
+CurseForgeGetModsResponse get_mods(request: CurseForgeGetModsByIdsListRequestBody)
-TResponse post(endpoint: str, body: TRequest)
-client: Client
}
class ModpackInfo {
+name: String
+minecraft_version: Option~String~
+mod_loader: Option~String~
+mod_loader_version: Option~String~
+modpack_type: String
+instance_id: Option~String~
}
class ModpackFile {
+url: String
+path: String
+size: Option~u64~
+sha1: Option~String~
}
class ParsedModpack {
+info: ModpackInfo
+files: Vec~ModpackFile~
+override_prefixes: Vec~String~
+ParsedModpack unknown(path: Path)
}
class CurseForgeGetModFilesRequestBody {
+file_ids: Vec~u64~
+CurseForgeGetModFilesRequestBody new(file_ids: Vec~u64~)
}
class CurseForgeGetModsByIdsListRequestBody {
+mod_ids: Vec~u64~
+filter_pc_only: Option~bool~
+CurseForgeGetModsByIdsListRequestBody new(mod_ids: Vec~u64~)
}
class CurseForgeGetFilesResponse {
+data: Vec~CurseForgeFile~
}
class CurseForgeGetModsResponse {
+data: Vec~CurseForgeMod~
}
class CurseForgeFile {
+id: u64
+game_id: u64
+mod_id: u64
+is_available: bool
+display_name: String
+file_name: String
+release_type: CurseForgeFileReleaseType
+file_status: CurseForgeFileStatus
+hashes: Vec~CurseForgeFileHash~
+file_date: String
+file_length: u64
+download_count: u64
+file_size_on_disk: Option~u64~
+download_url: Option~String~
+game_versions: Vec~String~
+sortable_game_versions: Vec~CurseForgeSortableGameVersion~
+dependencies: Vec~CurseForgeFileDependency~
+expose_as_alternative: Option~bool~
+parent_project_file_id: Option~u64~
+alternate_file_id: Option~u64~
+is_server_pack: Option~bool~
+server_pack_file_id: Option~u64~
+is_early_access_content: Option~bool~
+early_access_end_date: Option~String~
+file_fingerprint: u64
+modules: Vec~CurseForgeFileModule~
}
class CurseForgeMod {
+id: u64
+game_id: u64
+name: String
+slug: String
+links: CurseForgeModLinks
+summary: String
+status: CurseForgeModStatus
+download_count: u64
+is_featured: bool
+primary_category_id: u64
+categories: Vec~CurseForgeCategory~
+class_id: Option~u64~
+authors: Vec~CurseForgeModAuthor~
+logo: Option~CurseForgeModAsset~
+screenshots: Vec~CurseForgeModAsset~
+main_file_id: u64
+latest_files: Vec~CurseForgeFile~
+latest_files_indexes: Vec~CurseForgeFileIndex~
+latest_early_access_files_indexes: Vec~CurseForgeFileIndex~
+date_created: String
+date_modified: String
+date_released: String
+allow_mod_distribution: Option~bool~
+game_popularity_rank: u64
+is_available: bool
+thumbs_up_count: u64
+rating: Option~f64~
}
class CurseForgeHashAlgo {
<<enum>>
Sha1
Md5
}
class CurseForgeFileRelationType {
<<enum>>
EmbeddedLibrary
OptionalDependency
RequiredDependency
Tool
Incompatible
Include
}
class CurseForgeFileReleaseType {
<<enum>>
Release
Beta
Alpha
}
class CurseForgeFileStatus {
<<enum>>
Processing
ChangesRequired
UnderReview
Approved
Rejected
MalwareDetected
Deleted
Archived
Testing
Released
ReadyForReview
Deprecated
Baking
AwaitingPublishing
FailedPublishing
Cooking
Cooked
UnderManualReview
ScanningForMalware
ProcessingFile
PendingRelease
ReadyForCooking
PostProcessing
}
class CurseForgeModLoaderType {
<<enum>>
Any
Forge
Cauldron
LiteLoader
Fabric
Quilt
NeoForge
}
class CurseForgeModStatus {
<<enum>>
New
ChangesRequired
UnderSoftReview
Approved
Rejected
ChangesMade
Inactive
Abandoned
Deleted
UnderReview
}
ModpackApi --> ModpackParser : uses
ModpackApi --> ModpackFileResolver : uses
ModpackApi --> OverrideExtractor : uses
ZipModpackParser ..|> ModpackParser
ResolverChain ..|> ModpackFileResolver
CurseForgeFileResolver ..|> ModpackFileResolver
ZipOverrideExtractor ..|> OverrideExtractor
CurseForgeFileResolver --> CurseForgeApi : uses
CurseForgeFileResolver --> ModpackFile : produces
CurseForgeFileResolver --> CurseForgeFile : consumes
CurseForgeFileResolver --> CurseForgeMod : consumes
CurseForgeApi --> CurseForgeGetFilesResponse
CurseForgeApi --> CurseForgeGetModsResponse
CurseForgeGetFilesResponse --> CurseForgeFile
CurseForgeGetModsResponse --> CurseForgeMod
ParsedModpack --> ModpackInfo
ParsedModpack --> ModpackFile
CurseForgeFile --> CurseForgeFileHash
CurseForgeFile --> CurseForgeSortableGameVersion
CurseForgeFile --> CurseForgeFileDependency
CurseForgeFile --> CurseForgeFileModule
CurseForgeFile --> CurseForgeFileReleaseType
CurseForgeFile --> CurseForgeFileStatus
CurseForgeFileDependency --> CurseForgeFileRelationType
CurseForgeFileIndex --> CurseForgeFileReleaseType
CurseForgeFileIndex --> CurseForgeModLoaderType
CurseForgeMod --> CurseForgeModLinks
CurseForgeMod --> CurseForgeCategory
CurseForgeMod --> CurseForgeModAuthor
CurseForgeMod --> CurseForgeModAsset
CurseForgeMod --> CurseForgeFile
CurseForgeMod --> CurseForgeFileIndex
CurseForgeMod --> CurseForgeModStatus
CurseForgeFileHash --> CurseForgeHashAlgo
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
CurseForgeFileResolver::resolve, the resolvedmodpack.fileslist is replaced entirely with the resolved CurseForge files, so any non-CurseForge entries in the original modpack are dropped; consider merging the resolved CurseForge files back into the original list instead of overwriting it. - In
CurseForgeFileResolver::class_ids, all errors from the CurseForge API are swallowed and result in an emptyHashMap, which makes it hard to distinguish between 'no data' and 'request failed'; consider at least logging or otherwise surfacing the error while still allowing resolution to proceed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CurseForgeFileResolver::resolve`, the resolved `modpack.files` list is replaced entirely with the resolved CurseForge files, so any non-CurseForge entries in the original modpack are dropped; consider merging the resolved CurseForge files back into the original list instead of overwriting it.
- In `CurseForgeFileResolver::class_ids`, all errors from the CurseForge API are swallowed and result in an empty `HashMap`, which makes it hard to distinguish between 'no data' and 'request failed'; consider at least logging or otherwise surfacing the error while still allowing resolution to proceed.
## Individual Comments
### Comment 1
<location path="src-tauri/src/core/modpack/curseforge.rs" line_range="95-102" />
<code_context>
+ .await
+ .map_err(|e| format!("CurseForge API error: {e}"))?;
+
+ if !response.status().is_success() {
+ return Err(format!("CurseForge API returned {}", response.status()));
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Include response body when reporting non-successful CurseForge API responses.
Currently only the HTTP status code is surfaced, which makes diagnosing issues (rate limiting, auth, validation, etc.) difficult. Please also try to read and include the response body (possibly truncated and on a best-effort basis) in the error so callers get more useful diagnostics.
```suggestion
let response = self
.client
.post(format!("{CURSEFORGE_API_BASE_URL}{endpoint}"))
.header("x-api-key", api_key)
.json(body)
.send()
.await
.map_err(|e| format!("CurseForge API error: {e}"))?;
if !response.status().is_success() {
let status = response.status();
let body = response
.text()
.await
.unwrap_or_else(|e| format!("<failed to read response body: {e}>"));
// Truncate the body to a reasonable size to avoid huge error messages.
let max_len = 2048;
let body = if body.len() > max_len {
let truncated: String = body.chars().take(max_len).collect();
format!("{truncated}… [truncated to {max_len} chars]")
} else {
body
};
return Err(format!("CurseForge API returned {status}: {body}"));
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+95
to
+102
| let response = self | ||
| .client | ||
| .post(format!("{CURSEFORGE_API_BASE_URL}{endpoint}")) | ||
| .header("x-api-key", api_key) | ||
| .json(body) | ||
| .send() | ||
| .await | ||
| .map_err(|e| format!("CurseForge API error: {e}"))?; |
Contributor
There was a problem hiding this comment.
suggestion: Include response body when reporting non-successful CurseForge API responses.
Currently only the HTTP status code is surfaced, which makes diagnosing issues (rate limiting, auth, validation, etc.) difficult. Please also try to read and include the response body (possibly truncated and on a best-effort basis) in the error so callers get more useful diagnostics.
Suggested change
| let response = self | |
| .client | |
| .post(format!("{CURSEFORGE_API_BASE_URL}{endpoint}")) | |
| .header("x-api-key", api_key) | |
| .json(body) | |
| .send() | |
| .await | |
| .map_err(|e| format!("CurseForge API error: {e}"))?; | |
| let response = self | |
| .client | |
| .post(format!("{CURSEFORGE_API_BASE_URL}{endpoint}")) | |
| .header("x-api-key", api_key) | |
| .json(body) | |
| .send() | |
| .await | |
| .map_err(|e| format!("CurseForge API error: {e}"))?; | |
| if !response.status().is_success() { | |
| let status = response.status(); | |
| let body = response | |
| .text() | |
| .await | |
| .unwrap_or_else(|e| format!("<failed to read response body: {e}>")); | |
| // Truncate the body to a reasonable size to avoid huge error messages. | |
| let max_len = 2048; | |
| let body = if body.len() > max_len { | |
| let truncated: String = body.chars().take(max_len).collect(); | |
| format!("{truncated}… [truncated to {max_len} chars]") | |
| } else { | |
| body | |
| }; | |
| return Err(format!("CurseForge API returned {status}: {body}")); | |
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Rust backend’s modpack support by splitting the previous monolithic core/modpack.rs into a dedicated core/modpack/ module tree and extracting CurseForge networking/types into a separate API layer.
Changes:
- Introduces a
ModpackApifacade with pluggable parser/resolver/extractor components. - Adds dedicated format parsers (Modrinth/CurseForge/MultiMC), override extraction, and a resolver chain (including CurseForge file resolution).
- Extracts CurseForge API client + request/response/types into
core/modpack/curseforge.rsand adds basic modpack API tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src-tauri/src/core/modpack/types.rs | Defines shared public modpack data types (ModpackInfo, ModpackFile, ParsedModpack). |
| src-tauri/src/core/modpack/resolver.rs | Adds resolver abstractions and CurseForge file resolution implementation. |
| src-tauri/src/core/modpack/parser.rs | Adds parser abstraction and zip-based modpack parser entrypoint. |
| src-tauri/src/core/modpack/mod.rs | Introduces the new core::modpack module layout. |
| src-tauri/src/core/modpack/formats/multimc.rs | Implements MultiMC/PrismLauncher format parsing. |
| src-tauri/src/core/modpack/formats/modrinth.rs | Implements Modrinth .mrpack parsing and file list extraction. |
| src-tauri/src/core/modpack/formats/mod.rs | Adds format parser dispatch. |
| src-tauri/src/core/modpack/formats/curseforge.rs | Implements CurseForge manifest parsing producing placeholder curseforge:// URLs. |
| src-tauri/src/core/modpack/extractor.rs | Implements override extraction from zip archives with progress reporting. |
| src-tauri/src/core/modpack/curseforge.rs | New CurseForge API client + schema/types. |
| src-tauri/src/core/modpack/archive.rs | Centralizes zip open/read helpers. |
| src-tauri/src/core/modpack/api.rs | Adds the public ModpackApi facade, helper functions, and tests. |
| src-tauri/src/core/modpack.rs | Removes the previous monolithic modpack implementation (replaced by core/modpack/). |
Comment on lines
+1
to
+2
| #![allow(dead_code)] | ||
|
|
There was a problem hiding this comment.
Using #![allow(dead_code)] at the module root suppresses dead-code warnings for the entire modpack module (including future additions), which can hide genuinely unused/forgotten code. Prefer removing this, or scoping #[allow(dead_code)] to only the specific items that are intentionally unused (or gating with cfg/test).
Suggested change
| #![allow(dead_code)] |
Comment on lines
+71
to
+78
| impl Default for ModpackApi { | ||
| fn default() -> Self { | ||
| Self::with_components( | ||
| ZipModpackParser::default(), | ||
| ResolverChain::default(), | ||
| ZipOverrideExtractor::default(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
ModpackApi::default() eagerly constructs ResolverChain::default(), which in turn constructs a CurseForgeApi::default() (creating a new reqwest::Client). This means even metadata-only calls like the detect() helper will pay the cost of building the network client and resolver chain. Consider splitting construction so detect() uses only ZipModpackParser (or lazily initializes the resolver/client only when import() is called and the pack type requires it).
Member
代码审查发现的问题1. parser.rs - 错误静默处理解析失败时返回 unknown 类型继续处理，用户根本不知道出问题了 2. resolver.rs - API 失败静默返回空class_ids 失败时返回空 HashMap，导致 resourcepacks/shaderpacks 类的文件被错误分类到 mods 目录 3. curseforge.rs - 枚举反序列化无 fallbackAPI 返回未知值时直接 panic 4. extractor.rs - 路径 traversal guard 不完善Windows 路径规范化差异可能导致绕过 我已经准备好了修复补丁，等网络恢复后可以提交 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
HsiangNianian
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a modular modpack core with support for parsing, importing, and extracting multiple modpack formats, including CurseForge integration.
New Features:
Enhancements: