Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a new "Demo Mode" account type, allowing users to try Minecraft without a purchased account. Changes include the addition of DemoAccount and DemoAccountFactory classes, UI updates to the account creation and Microsoft login screens, and updated localization files. Review feedback suggests improving the UI layout for demo hints, registering the new factory for better visibility, standardizing account identifiers, correcting a mismatch between code and comments regarding authentication types, and ensuring the equals and hashCode methods are robust and consistent.
HMCL/src/main/java/org/jackhuang/hmcl/ui/account/CreateAccountPane.java
Outdated
Show resolved
Hide resolved
| public static final DemoAccountFactory FACTORY_DEMO = new DemoAccountFactory(AUTHLIB_INJECTOR_DOWNLOADER); | ||
| public static final AuthlibInjectorAccountFactory FACTORY_AUTHLIB_INJECTOR = new AuthlibInjectorAccountFactory(AUTHLIB_INJECTOR_DOWNLOADER, Accounts::getOrCreateAuthlibInjectorServer); | ||
| public static final MicrosoftAccountFactory FACTORY_MICROSOFT = new MicrosoftAccountFactory(new MicrosoftService(OAUTH_CALLBACK)); | ||
| public static final List<AccountFactory<?>> FACTORIES = immutableListOf(FACTORY_OFFLINE, FACTORY_MICROSOFT, FACTORY_AUTHLIB_INJECTOR); |
There was a problem hiding this comment.
FACTORY_DEMO 应该被添加到 FACTORIES 列表中。如果不添加，该账户类型将不会出现在“添加账户”对话框的选项卡中，用户只能通过微软登录界面的快捷按钮发现此功能，这降低了功能的发现感和一致性。
| public static final List<AccountFactory<?>> FACTORIES = immutableListOf(FACTORY_OFFLINE, FACTORY_MICROSOFT, FACTORY_AUTHLIB_INJECTOR); | |
| public static final List<AccountFactory<?>> FACTORIES = immutableListOf(FACTORY_OFFLINE, FACTORY_MICROSOFT, FACTORY_AUTHLIB_INJECTOR, FACTORY_DEMO); |
HMCLCore/src/main/java/org/jackhuang/hmcl/auth/demo/DemoAccount.java
Outdated
Show resolved
Hide resolved
|
|
||
| public AuthInfo logIn() throws AuthenticationException { | ||
| // Using "legacy" user type here because "mojang" user type may cause "invalid session token" or "disconnected" when connecting to a game server. | ||
| return new DemoAuthInfo(username, uuid, UUIDTypeAdapter.fromUUID(UUID.randomUUID()), AuthInfo.USER_TYPE_MSA, "{}"); |
There was a problem hiding this comment.
代码实现与注释描述不符。注释提到使用 "legacy" 用户类型以避免连接服务器时的验证问题，但代码中实际使用的是 AuthInfo.USER_TYPE_MSA ("msa")。建议根据注释意图将其改为 AuthInfo.USER_TYPE_LEGACY。
| return new DemoAuthInfo(username, uuid, UUIDTypeAdapter.fromUUID(UUID.randomUUID()), AuthInfo.USER_TYPE_MSA, "{}"); | |
| return new DemoAuthInfo(username, uuid, UUIDTypeAdapter.fromUUID(UUID.randomUUID()), AuthInfo.USER_TYPE_LEGACY, "{}"); |
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return username.hashCode(); |
There was a problem hiding this comment.
hashCode 的实现应包含 isPortable() 属性，以保持与 equals 方法的一致性。此外，建议同时包含 uuid 以增强唯一性校验。
| return username.hashCode(); | |
| return java.util.Objects.hash(isPortable(), username, uuid); |
| if (!(obj instanceof DemoAccount)) | ||
| return false; | ||
| DemoAccount another = (DemoAccount) obj; | ||
| return isPortable() == another.isPortable() && username.equals(another.username); |
There was a problem hiding this comment.
在 equals 方法中，建议同时校验 uuid 属性，以确保账户的唯一性判断更加严谨。
| return isPortable() == another.isPortable() && username.equals(another.username); | |
| return isPortable() == another.isPortable() && username.equals(another.username) && uuid.equals(another.uuid); |
|
这个功能有意义吗？ |
有restrict offline，为什么没意义 |
|
3 participants
No description provided.