Skip to content

Comments

feat(instance): add&delete game server through SJMCL#1328

Merged
UNIkeEN merged 9 commits intoUNIkeEN:mainfrom
hbz114514:feat-issue-1169
Feb 22, 2026
Merged

feat(instance): add&delete game server through SJMCL#1328
UNIkeEN merged 9 commits intoUNIkeEN:mainfrom
hbz114514:feat-issue-1169

Conversation

@hbz114514
Copy link
Contributor

@hbz114514 hbz114514 commented Jan 21, 2026

Checklist

  • Changes have been tested locally and work as expected.
  • All tests in workflows pass successfully.
  • Documentation has been updated if necessary.
  • Code formatting and commit messages align with the project's conventions.
  • Comments have been added for any complex logic or functionality if possible.

This PR is a ..

  • 🆕 New feature
  • 🐞 Bug fix
  • 🛠 Refactoring
  • ⚡️ Performance improvement
  • 🌐 Internationalization
  • 📄 Documentation improvement
  • 🎨 Code style optimization
  • ❓ Other (Please specify below)

Related Issues

partly fix #1169

Description

  • Please insert your description here and provide info about the "what" this PR is solving.

Additional Context

11 12

	modified:   src-tauri/src/instance/models/misc.rs
	modified:   src-tauri/src/lib.rs
	modified:   src/enums/service-error.ts
	modified:   src/locales/en.json
	modified:   src/locales/zh-Hans.json
	modified:   src/pages/instances/details/[id]/worlds.tsx
	modified:   src/services/instance.ts
@hbz114514
Copy link
Contributor Author

改完了(大概((
暂时没有加删除按钮(因为加上感觉很挤
把多余空行删掉了,错误内容放到了翻译文件的service部分,label和placeholder也修改了,把原来写在modal里的内容合并到了worlds.tsx里,新开一个分支重新提交PR,无conflict

@hbz114514
Copy link
Contributor Author

我去把上一个关掉

@hbz114514
Copy link
Contributor Author

添加了删除按钮和删除命令

@hbz114514
Copy link
Contributor Author

感谢在溪不然我都忘了(

@hbz114514
Copy link
Contributor Author

还要感谢在溪的部分代码(真的很有帮助(


let mut existing_servers = load_servers_info_from_path(&servers_dat_path).await?;

let original_len = existing_servers.len();
Copy link
Owner

Choose a reason for hiding this comment

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

这个变量用到了吗,还有这个函数为啥写一行空一行

Ok(())
}

async fn save_servers_to_nbt(path: &PathBuf, servers: &[GameServerInfo]) -> SJMCLResult<()> {
Copy link
Owner

Choose a reason for hiding this comment

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

请不要在 commands.rs 写非 command 函数

Copy link
Contributor Author

Choose a reason for hiding this comment

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

移动到helpers中了

import { UNIXToISOString, formatRelativeTime } from "@/utils/datetime";
import { base64ImgSrc } from "@/utils/string";

const DeleteServerDialog: React.FC<{
Copy link
Owner

Choose a reason for hiding this comment

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

请使用 openGenericConfirmDialog

});
}
} catch (error) {
toast({
Copy link
Owner

Choose a reason for hiding this comment

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

不必要的错误 catch


if (response.status === "success") {
toast({
title: t("InstanceWorldsPage.serverList.deleteSuccess"),
Copy link
Owner

Choose a reason for hiding this comment

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

service 函数的 toast 无需定义 key

<Empty withIcon={false} size="sm" />
)}
</Section>
<Modal isOpen={isAddServerModalOpen} onClose={onAddServerModalClose}>
Copy link
Owner

Choose a reason for hiding this comment

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

本项目中的所有复杂逻辑 modal 应该均为独立组件,且包含 handle 逻辑

刷新逻辑应作为 onSuccess 传入

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经将输入弹窗独立为函数组件
小问题:使用onSuccess时,短时间内添加多个服务器会导致重复刷新,有概率出现列表显示错误(目前还不知道什么原因),所以现在没有用onSuccess,而是在成功之后直接刷新,测试没有问题。

Copy link
Owner

Choose a reason for hiding this comment

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

已经将输入弹窗独立为函数组件 小问题:使用onSuccess时,短时间内添加多个服务器会导致重复刷新,有概率出现列表显示错误(目前还不知道什么原因),所以现在没有用onSuccess,而是在成功之后直接刷新,测试没有问题。

我没有看到此条 PR 在 modals/ 下新增了 add-server-modal.tsx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是要单独放到\src\components\modals里面吗

autoFocus
/>
</FormControl>
<FormControl isRequired>
Copy link
Owner

Choose a reason for hiding this comment

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

isRequired 的 FormControl 是否需要添加 FormErrMsg,请参考其他 input 类 modal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

添加了FormErrMsg,调整了服务器地址和服务器名称的顺序

	modified:   src-tauri/src/instance/helpers/server.rs
	modified:   src/locales/en.json
	modified:   src/locales/zh-Hans.json
	modified:   src/pages/instances/details/[id]/worlds.tsx
@hbz114514 hbz114514 changed the title feat(instance): add game server through SJMCL feat(instance): add&delete game server through SJMCL Jan 28, 2026
"offline": "Offline"
},
"delete": "Delete this Server",
"deleteConfirm": {
Copy link
Owner

Choose a reason for hiding this comment

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

请与项目内现有删除 modal 的代码与翻译风格保持一致

Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修改为deletegameserveralertdialog部分

handleRetrieveGameServerList(false);
handleRetrieveGameServerList(true);

// refresh every minute to query server info
Copy link
Owner

Choose a reason for hiding this comment

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

没看懂这行注释为什么要删除

Copy link
Contributor Author

Choose a reason for hiding this comment

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

补回去了

import { UNIXToISOString, formatRelativeTime } from "@/utils/datetime";
import { base64ImgSrc } from "@/utils/string";

function AddGameServerModal(props: any) {
Copy link
Owner

Choose a reason for hiding this comment

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

  1. 与现有项目内所有 modal 定义方式与代码文件结构不一致
  2. Typescript 项目请勿使用 any

Copy link
Contributor Author

Choose a reason for hiding this comment

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

按照worldleveldata的格式改了一下,现在应该没问题((?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

globalsharedmodal和其他modal区别在能否用opensharedmodal调用吗((上次没问清,我的问题

btnOK: t("General.delete"),
isAlert: true,
onOKCallback: () => {
setServerToDelete(server);
Copy link
Owner

Choose a reason for hiding this comment

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

这个 state 不是一定需要的吧(?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

哪一个(
如果是isAlert的话,删除服务器的行为是不是按钮也应该是红的(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

删掉了,现在弹窗的是蓝色按钮了(

serverName={serverName}
onNameChange={(e: any) => setServerName(e.target.value)}
serverAddress={serverAddress}
onAddressChange={(e: any) => setServerAddress(e.target.value)}
Copy link
Owner

Choose a reason for hiding this comment

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

请考虑这么多参数的设计是否合理,相同类型的 AddAuthServerModal 只需要传入 isOpen 和 onClose

	modified:   src/locales/en.json
	modified:   src/locales/zh-Hans.json
	modified:   src/pages/instances/details/[id]/worlds.tsx
@hbz114514
Copy link
Contributor Author

修改了addGameServerModal ,独立为modal文件夹下的文件,将国际化内容放到worldleveldatamodal后边了(应该没问题
onSuccess也是老问题,所以刷新逻辑一并放到onclose中

@UNIkeEN
Copy link
Owner

UNIkeEN commented Feb 11, 2026

将国际化内容放到worldleveldatamodal后边了(应该没问题

可是国际化内容最外层是按照字母序排序的啊

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds functionality to add and delete Minecraft game servers directly from the SJMCL launcher interface, addressing issue #1169. Users can now manage their server list through a dedicated modal dialog without manually editing the servers.dat file.

Changes:

  • Implements Rust backend commands (add_game_server, delete_game_server) to manipulate the servers.dat NBT file
  • Creates a new frontend modal (AddGameServerModal) for adding servers with address and name fields
  • Integrates server management UI into the instance worlds page with add/delete/launch buttons
  • Adds comprehensive localization support for both English and Chinese languages

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src-tauri/src/lib.rs Registers new add_game_server and delete_game_server commands
src-tauri/src/instance/commands.rs Implements add/delete server commands with duplicate checking and NBT file operations
src-tauri/src/instance/helpers/server.rs Adds NBT conversion traits, path helper, and save function for server data
src-tauri/src/instance/models/misc.rs Adds DuplicateServer and FileOperationError enum variants
src/services/instance.ts Exposes TypeScript service methods for add/delete game server operations
src/components/modals/add-game-server-modal.tsx New modal component for adding game servers with validation
src/pages/instances/details/[id]/worlds.tsx Integrates add/delete server UI with confirmation dialogs and auto-refresh
src/enums/service-error.ts Adds DuplicateServer and FileOperationError to frontend error enum
src/locales/en.json English translations for server management UI and error messages
src/locales/zh-Hans.json Chinese translations for server management UI and error messages

Comment on lines +444 to +461
pub async fn delete_game_server(
app: AppHandle,
instance_id: String,
server_addr: String,
) -> SJMCLResult<()> {
let nbt_path = match get_servers_nbt_path_by_instance_id(&app, &instance_id) {
Some(path) => path,
None => return Err(InstanceError::InstanceNotFoundByID.into()),
};
let mut existing_servers = load_servers_info_from_nbt(&nbt_path).await?;

existing_servers.retain(|server| server.ip != server_addr);
save_servers_to_nbt(&nbt_path, &existing_servers)
.await
.map_err(|_| InstanceError::FileOperationError)?;

Ok(())
}

This comment was marked as resolved.

Comment on lines +249 to +255
DuplicateServer,
FileNotFoundError,
InvalidSourcePath,
FileCreationFailed,
FileCopyFailed,
FileMoveFailed,
FileOperationError,
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The localization files define a SERVER_NOT_FOUND error message but the corresponding error variant is missing from the InstanceError enum. Either add the ServerNotFound variant to the enum (and use it in the delete_game_server command), or remove the unused error message from the localization files. For consistency with the error messages defined, adding the variant is recommended.

Copilot uses AI. Check for mistakes.
Comment on lines 93 to 94
const urlObj = new URL(presetUrl);
const extractedName = urlObj.hostname || presetUrl;

This comment was marked as off-topic.

</FormLabel>
<Input
id="serverUrl"
type="url"
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The input type "url" is semantically incorrect for Minecraft server addresses. These addresses are typically in the format "example.com" or "example.com:25565", which are not valid URLs (they lack a protocol like http:// or https://). Using type="url" may cause browser validation issues and confuse users.

Change the input type to "text" to properly accept Minecraft server address formats.

Suggested change
type="url"
type="text"

Copilot uses AI. Check for mistakes.
(instanceId: string) => {
const finalServerName = serverName.trim() || serverNamePlaceholder;
setIsLoading(true);
InstanceService.addGameServer(instanceId, serverUrl, finalServerName)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The server address is not trimmed before being sent to the backend. If users accidentally include leading or trailing whitespace in the address field, it will be saved as-is, which could cause connection issues and duplicate entries (since "example.com" and " example.com" would be treated as different servers).

Consider trimming the serverUrl before sending it: InstanceService.addGameServer(instanceId, serverUrl.trim(), finalServerName)

Suggested change
InstanceService.addGameServer(instanceId, serverUrl, finalServerName)
InstanceService.addGameServer(instanceId, serverUrl.trim(), finalServerName)

Copilot uses AI. Check for mistakes.
}, 60000);
return () => clearInterval(intervalId);
}, [instanceId, handleRetrieveGameServerList]);
}, [instanceId, handleRetrieveGameServerList, refreshGameServerList]);
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

The useEffect dependency array includes both handleRetrieveGameServerList and refreshGameServerList, but since refreshGameServerList only depends on handleRetrieveGameServerList, including both is redundant. The effect should depend only on the primitive dependency (if needed at all).

However, in this case, since the interval only calls handleRetrieveGameServerList(true) directly, the dependency array could be simplified to just [handleRetrieveGameServerList] and remove the refreshGameServerList() call from the effect body (or keep both but remove refreshGameServerList from the dependency array).

Suggested change
}, [instanceId, handleRetrieveGameServerList, refreshGameServerList]);
}, [handleRetrieveGameServerList]);

Copilot uses AI. Check for mistakes.
@hbz114514
Copy link
Contributor Author

测试没有问题。可以正常添加删除服务器,并且成功后会刷新列表。默认服务器名称正常。

@UNIkeEN UNIkeEN merged commit cdd9a7d into UNIkeEN:main Feb 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] 支持实例服务器列表的编辑

2 participants