Skip to content

Conversation

@1zun4
Copy link
Member

@1zun4 1zun4 commented May 5, 2025

image Screenshot_20250720_040230

Needs to be improved by @SenkJu - if required, revert commit: f463011

@1zun4 1zun4 added this to the 0.31.0 milestone May 5, 2025
@1zun4 1zun4 requested a review from SenkJu May 5, 2025 02:41
@1zun4 1zun4 modified the milestones: 0.31.0, 0.32.0 Jun 3, 2025
@1zun4 1zun4 marked this pull request as draft June 3, 2025 19:33
1zun4 added 3 commits July 20, 2025 03:26
# Conflicts:
#	src/main/kotlin/net/ccbluex/liquidbounce/event/EventManager.kt
#	src/main/kotlin/net/ccbluex/liquidbounce/features/command/commands/client/client/CommandClientAccountSubcommand.kt
@1zun4 1zun4 marked this pull request as ready for review July 20, 2025 02:03
@1zun4 1zun4 modified the milestones: 0.32.0, 0.33.0, 0.32.1 Aug 24, 2025
@1zun4 1zun4 modified the milestones: 0.34.0, 0.35.0 Oct 10, 2025
@1zun4 1zun4 removed this from the 0.35.0 milestone Nov 8, 2025
@1zun4 1zun4 marked this pull request as draft November 8, 2025 17:50
@CCBlueX CCBlueX deleted a comment from Copilot AI Nov 11, 2025
@1zun4 1zun4 requested a review from Copilot November 11, 2025 23:03
Copy link

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 account management functionality to LiquidBounce, enabling users to log in and out of their LiquidBounce accounts through both REST API endpoints and CLI commands. The implementation includes WebSocket event notifications for login/logout actions and a new UI component to display user information.

  • Added REST API endpoints for user authentication (GET /api/v1/client/user, POST /api/v2/client/user/login, POST /api/v2/client/user/logout)
  • Refactored OAuth server implementation to improve cleanup and error handling
  • Enhanced user information model to include additional fields (email, name, nickname, groups, admin status)

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
UserFunctions.kt New REST API endpoints for user account operations (get, login, logout)
InteropFunctionRegistry.kt Registered the new user-related API endpoints
CommandClientAccountSubcommand.kt Updated CLI command to fire events and display extended user info
ClientEvents.kt Added UserLoggedInEvent and UserLoggedOutEvent events
EventManager.kt Registered the new user events in the event class list
OAuthClient.kt Refactored OAuth server lifecycle and improved error handling
UserInformation.kt Extended data model with additional user fields
ClientAccount.kt Added @JvmField annotation to EMPTY_ACCOUNT
UserBox.svelte New UI component for displaying user information and login/logout controls
Title.svelte Integrated UserBox component into the title screen
types.ts Added ClientUser interface definition
rest.ts Added API client functions for user operations
events.ts Added WebSocket event types for user login/logout

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +39 to +42
get("/user", ::getUser).apply {
post("/login", ::loginUser)
post("/logout", ::logoutUser)
}
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The comment at line 61 says POST /api/v2/client/user/login but the route is registered under /api/v1/client. This creates a version mismatch. Either update the comment in UserFunctions.kt or ensure the endpoints are registered under the correct API version.

Copilot uses AI. Check for mistakes.
chat(regular("Nickname: "), variable(info.nickname))
chat(regular("Email: "), variable(info.email))
chat(regular("User ID: "), variable(info.userId))
chat(regular("Groups: "), info.groups.map(::variable).joinToText(", ".asText()))
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The joinToText function with separator is not a standard Kotlin collection function. This should likely be joinToString with the separator as a string, or the text concatenation logic needs verification to ensure it produces the expected output format.

Suggested change
chat(regular("Groups: "), info.groups.map(::variable).joinToText(", ".asText()))
chat(regular("Groups: "), info.groups.map(::variable).joinToString(", "))

Copilot uses AI. Check for mistakes.

val bootstrap = ServerBootstrap()
.group(bossGroup, workerGroup)
.channelFactory(::NioServerSocketChannel)
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Using .channelFactory(::NioServerSocketChannel) is unconventional for Netty's ServerBootstrap. The standard approach is .channel(NioServerSocketChannel::class.java) which was used in the original code. This change may cause issues with channel instantiation.

Suggested change
.channelFactory(::NioServerSocketChannel)
.channel(NioServerSocketChannel::class.java)

Copilot uses AI. Check for mistakes.
userId: string;
email: string;
name: string | null;
nickname: string | null;
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The nickname field is typed as string | null in TypeScript but the Kotlin UserInformation model defines it as non-nullable String. This type inconsistency will cause runtime errors when a user has no nickname. Either make the Kotlin field nullable or update the TypeScript type to match.

Suggested change
nickname: string | null;
nickname: string;

Copilot uses AI. Check for mistakes.
Comment on lines +55 to +60
onMount(() => {
const timeout = setTimeout(() => {
if (loading) loading = false;
}, 3000);
return () => clearTimeout(timeout);
});
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

There are two separate onMount calls (lines 41 and 55). These should be combined into a single onMount block to avoid confusion and ensure proper initialization order.

Copilot uses AI. Check for mistakes.
Comment on lines +720 to +734
await fetch(`${API_BASE}/client/user/login`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
}

export async function logoutClientUser() {
await fetch(`${API_BASE}/client/user/logout`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The loginClientUser and logoutClientUser functions don't check the response status or throw errors on failure. They should validate the response and throw meaningful errors to help debug authentication issues, similar to how getClientUser handles errors.

Suggested change
await fetch(`${API_BASE}/client/user/login`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
}
export async function logoutClientUser() {
await fetch(`${API_BASE}/client/user/logout`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
const response = await fetch(`${API_BASE}/client/user/login`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
if (!response.ok) {
throw new Error(`Failed to login client user: ${response.status} ${response.statusText}`);
}
}
export async function logoutClientUser() {
const response = await fetch(`${API_BASE}/client/user/logout`, {
method: "POST",
headers: {
"Content-Type": "application/json"
}
});
if (!response.ok) {
throw new Error(`Failed to logout client user: ${response.status} ${response.statusText}`);
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants