[1.20.4] Ref: Network, authentication and discord #106
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
common/src/main/kotlin/com/lambda/network/api/v1/endpoints/JoinParty.kt (1)
36-41:⚠️ Potential issueAdd error handling for the API response.
The current implementation directly accesses the third element of the response tuple without any error handling. This could lead to runtime exceptions if the request fails.
-) = - Fuel.put("${apiUrl}/api/${apiVersion.value}/party/join", listOf("id" to partyId)) - .jsonBody("""{ "id": "$partyId" }""") - .authentication() - .bearer(Network.accessToken) - .responseObject<Party>().third +) { + require(partyId.isNotBlank()) { "Party ID cannot be empty" } + require(partyId.matches("[a-zA-Z0-9]+".toRegex())) { "Invalid party ID format" } + + val (request, response, result) = Fuel.put("${apiUrl}/api/${apiVersion.value}/party/join") + .jsonBody("""{ "id": "$partyId" }""") + .authentication() + .bearer(Network.accessToken) + .responseObject<Party>() + + return when (result) { + is com.github.kittinunf.result.Result.Success -> result.value + is com.github.kittinunf.result.Result.Failure -> { + val ex = result.error + throw IllegalStateException("Failed to join party: ${ex.message}", ex) + } + } +}
🧹 Nitpick comments (18)
common/src/main/kotlin/com/lambda/event/listener/SafeListener.kt (1)
164-164: Consider making the predicate parameter required.The default value of
{ true }for the predicate means the listener will unsubscribe after the first event by default. This behavior might not be obvious to users and could lead to unexpected unsubscriptions.Consider removing the default value to make the predicate parameter required:
- noinline predicate: SafeContext.(T) -> Boolean = { true }, + noinline predicate: SafeContext.(T) -> Boolean,common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LeaveParty.kt (1)
28-32: LGTM! Clean implementation with centralized configuration.The function demonstrates good practices:
- Uses centralized configuration from Network module
- Implements proper bearer token authentication
- Maintains type safety with Party model
Consider adding error handling.
The current implementation might benefit from explicit error handling for network failures and API errors.
Consider wrapping the response in a Result type:
-fun leaveParty() = +fun leaveParty(): Result<Party> = runCatching { Fuel.put("${apiUrl}/api/${apiVersion.value}/party/leave") .authentication() .bearer(Network.accessToken) - .responseObject<Party>().third + .responseObject<Party>().third.get() +}common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LinkDiscord.kt (2)
2-2: Update the copyright year.The copyright year is set to 2025, which is in the future. Please update it to the current year (2024) or the year when the code was created.
- * Copyright 2025 Lambda + * Copyright 2024 Lambda
29-33: Validate the Discord token format.The function accepts any string as a Discord token without validation. Consider adding input validation to ensure the token matches the expected format.
fun linkDiscord( // The player's Discord token. // example: OTk1MTU1NzcyMzYxMTQ2NDM4 discordToken: String, -) = +) { + require(discordToken.matches(Regex("^[A-Za-z0-9_-]+$"))) { "Invalid Discord token format" }common/src/main/kotlin/com/lambda/network/api/v1/endpoints/GetParty.kt (1)
28-32: Consider handling errors more explicitly.
Currently, only thethirdcomponent of theFuelresponse triple is returned, which can obscure potential network, parsing, or authentication errors. Providing more explicit error handling or returning a richer result could improve debugging and resilience.Here’s a sample refactor that captures both success and error cases:
-fun getParty() = - Fuel.get("${apiUrl}/api/${apiVersion.value}/party") - .authentication() - .bearer(Network.accessToken) - .responseObject<Party>().third +fun getParty(): Party? { + val (_, _, result) = Fuel.get("${apiUrl}/api/${apiVersion.value}/party") + .authentication() + .bearer(Network.accessToken) + .responseObject<Party>() + + return result.fold( + { party -> party }, + { error -> + // TODO: log error or rethrow + null + } + ) +}Would you like me to open a new ticket and provide a more detailed implementation for error handling and token refresh logic?
common/src/main/kotlin/com/lambda/Lambda.kt (1)
83-114: Consider adding SSE error handling and status code checks.
This block currently does not handle non-200 responses or exceptions that may occur when establishing the SSE connection. Additionally, because SSE streams can potentially stay open indefinitely, the call tolatch.await()may block the thread if the server never closes the connection or fails mid-stream.Below is an example diff to early-return on non-200 responses:
client.sendAsync(request, HttpResponse.BodyHandlers.ofLines()) .thenAccept { response -> + if (response.statusCode() != 200) { + LOG.error("SSE responded with status code: ${response.statusCode()}") + latch.countDown() + return@thenAccept + } println("Connected to SSE stream") response.body().forEach { line -> if (line.startsWith("data:")) { val data = line.substring(5).trim() println("Received event data: $data") } } latch.countDown() }common/src/main/kotlin/com/lambda/network/api/v1/endpoints/PartyUpdates.kt (1)
1-47: Introduce better error handling for SSE and JSON parsing.
Currently, if the server responds with a non-200 status code or sends malformed JSON, the function may fail silently or terminate due to an unhandled exception inLambda.gson.fromJson(...). Consider verifying the response status code and surrounding the JSON parsing with a try/catch block to mitigate unhandled exceptions.Example snippet to handle non-200 statuses and catch JSON parse errors:
.thenAccept { response -> + if (response.statusCode() != 200) { + Lambda.LOG.error("SSE responded with status code: ${response.statusCode()}") + return@thenAccept + } response.body().forEach { if (!it.startsWith("data:")) return@forEach val data = it.substring(5).trim() + try { val party = Lambda.gson.fromJson(data, Party::class.java) block(party) + } catch (e: Exception) { + Lambda.LOG.error("Failed to parse SSE data: $data", e) + } } }common/src/main/kotlin/com/lambda/network/api/v1/endpoints/JoinParty.kt (2)
23-24: Remove duplicate import forjsonBody.There are two imports for
jsonBody. Since you're using the JSON string version in the code, you can remove the Gson-specific import:import com.github.kittinunf.fuel.core.extensions.jsonBody -import com.github.kittinunf.fuel.gson.jsonBody
36-38: Avoid sending duplicate data in query params and request body.The party ID is being sent both as a query parameter and in the request body. Choose one method, preferably the request body for POST/PUT requests.
- Fuel.put("${apiUrl}/api/${apiVersion.value}/party/join", listOf("id" to partyId)) - .jsonBody("""{ "id": "$partyId" }""") + Fuel.put("${apiUrl}/api/${apiVersion.value}/party/join") + .jsonBody("""{ "id": "$partyId" }""")common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (2)
55-57: Consider secure storage or caching forapiAuth.
Storing the authentication object in memory can be risky if it persists for long sessions. Additionally, theTODO: Cachecomment indicates there are plans for a more robust solution. Consider implementing secure storage or encryption to preserve user credentials.
92-107: Handle connection failures gracefully.
The call toClientConnection.connect(addr, ...)followed bysyncUninterruptibly()blocks until completion. If it fails, the user might be stuck or experience a crash. Implement error handling or fallback logic to manage unreachable servers or connection timeouts.common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (2)
120-128: Add user feedback for successful party join.
When joining a party is successful, consider providing some positive confirmation (e.g., “You joined the party!”). This helps confirm that everything worked.
195-214: Consider dynamic button usage or fallback.
When no party is available, a "Download" button is displayed. If the user already has the client installed, they might not need this link each time. Consider tailoring the fallback button or providing a different action to improve user experience.common/src/main/kotlin/com/lambda/network/api/v1/endpoints/Login.kt (1)
27-35: Documentation could be enhanced with response details.While the parameter documentation is good, it would be helpful to include:
- Expected response format
- Possible error scenarios
- Rate limiting details if any
common/src/main/java/com/lambda/mixin/network/ClientPlayNetworkHandlerMixin.java (1)
34-37: Consider validating packet data before posting event.The event is posted without validating the packet data. While this might be acceptable for join events, consider adding basic validation or error handling to prevent potential issues.
@Inject(method = "onGameJoin(Lnet/minecraft/network/packet/s2c/play/GameJoinS2CPacket;)V", at = @At("TAIL")) void injectJoinPacket(GameJoinS2CPacket packet, CallbackInfo ci) { + if (packet == null) return; EventFlow.post(new WorldEvent.Join()); }common/src/main/kotlin/com/lambda/event/events/WorldEvent.kt (1)
39-41: Simplify class declaration and document event placement.
- The empty constructor can be simplified.
- The TODO comment suggests uncertainty about event placement. Consider moving this event to a more specific event class like
ConnectionEventorPlayerEventif it's more appropriate.- class Join() : Event + object Join : Event🧰 Tools
🪛 detekt (1.23.7)
[warning] 41-41: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
common/src/main/kotlin/com/lambda/util/extension/Structures.kt (1)
192-205: Optimize bit manipulation logic for better readability and performance.The bit manipulation logic is complex and could be refactored for better readability and performance.
Consider extracting the bit manipulation logic into a separate function:
+private fun extractStateId(palette: LongArray, index: Int, bits: Int, maxEntryValue: Long): Long { + val startOffset = index * bits + val startArrIndex = startOffset / 64 + val endArrIndex = ((index + 1) * bits - 1) / 64 + val startBitOffset = startOffset % 64 + + return if (startArrIndex == endArrIndex) { + palette[startArrIndex] ushr startBitOffset and maxEntryValue + } else { + (palette[startArrIndex] ushr startBitOffset or + palette[endArrIndex] shl (64 - startBitOffset)) and maxEntryValue + } +} -val startOffset = index * bits -val startArrIndex = startOffset / 64 -val endArrIndex = ((index + 1) * bits - 1) / 64 -val startBitOffset = startOffset % 64 - -val stateId = - if (startArrIndex == endArrIndex) palette[startArrIndex] ushr startBitOffset and maxEntryValue - else (palette[startArrIndex] ushr startBitOffset or palette[endArrIndex] shl (64 - startBitOffset)) and maxEntryValue +val stateId = extractStateId(palette, index, bits, maxEntryValue)common/src/main/kotlin/com/lambda/event/listener/UnsafeListener.kt (1)
193-208: Add error handling and cancellation support for coroutines.While the coroutine support is a good addition, consider adding error handling and cancellation support.
Consider this implementation:
val listener = UnsafeListener<T>(priority, this, alwaysListen) { event -> - runConcurrent(scheduler) { - function(event) - } + runConcurrent(scheduler) { + try { + function(event) + } catch (e: CancellationException) { + // Handle cancellation if needed + throw e + } catch (e: Exception) { + // Log or handle other exceptions + // Consider whether to rethrow or suppress + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
build.gradle.kts(2 hunks)common/src/main/java/com/lambda/mixin/network/ClientPlayNetworkHandlerMixin.java(2 hunks)common/src/main/java/com/lambda/mixin/world/ClientWorldMixin.java(2 hunks)common/src/main/kotlin/com/lambda/Lambda.kt(5 hunks)common/src/main/kotlin/com/lambda/command/commands/DiscordCommand.kt(1 hunks)common/src/main/kotlin/com/lambda/command/commands/RpcCommand.kt(0 hunks)common/src/main/kotlin/com/lambda/event/events/WorldEvent.kt(1 hunks)common/src/main/kotlin/com/lambda/event/listener/SafeListener.kt(3 hunks)common/src/main/kotlin/com/lambda/event/listener/UnsafeListener.kt(3 hunks)common/src/main/kotlin/com/lambda/http/api/rpc/v1/endpoints/UpdateParty.kt(0 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)common/src/main/kotlin/com/lambda/module/modules/client/DiscordRPC.kt(0 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Network.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/CreateParty.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/DeleteParty.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/GetParty.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/JoinParty.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LeaveParty.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LinkDiscord.kt(2 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/Login.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/PartyUpdates.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Authentication.kt(2 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Party.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Player.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Settings.kt(1 hunks)common/src/main/kotlin/com/lambda/util/extension/Nbt.kt(1 hunks)common/src/main/kotlin/com/lambda/util/extension/Structures.kt(3 hunks)common/src/main/kotlin/com/lambda/util/math/MathUtils.kt(1 hunks)common/src/main/resources/lambda.accesswidener(1 hunks)forge/build.gradle.kts(1 hunks)gradle.properties(1 hunks)
💤 Files with no reviewable changes (3)
- common/src/main/kotlin/com/lambda/command/commands/RpcCommand.kt
- common/src/main/kotlin/com/lambda/http/api/rpc/v1/endpoints/UpdateParty.kt
- common/src/main/kotlin/com/lambda/module/modules/client/DiscordRPC.kt
✅ Files skipped from review due to trivial changes (5)
- common/src/main/kotlin/com/lambda/network/api/v1/models/Settings.kt
- gradle.properties
- common/src/main/kotlin/com/lambda/network/api/v1/models/Party.kt
- common/src/main/java/com/lambda/mixin/world/ClientWorldMixin.java
- common/src/main/kotlin/com/lambda/network/api/v1/models/Player.kt
🧰 Additional context used
🪛 detekt (1.23.7)
common/src/main/kotlin/com/lambda/event/events/WorldEvent.kt
[warning] 41-41: An empty default constructor can be removed.
(detekt.empty-blocks.EmptyDefaultConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Lambda
🔇 Additional comments (24)
common/src/main/kotlin/com/lambda/event/listener/SafeListener.kt (2)
133-160: LGTM! Documentation accurately reflects the simplified event handling.The updated documentation clearly explains the predicate-based unsubscription mechanism and provides a helpful usage example.
161-165: LGTM! Simplified function signature improves type safety and usability.The removal of the transformation logic and consistent use of type parameter
Tmakes the code more focused and type-safe.common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LeaveParty.kt (2)
18-18: LGTM! Package restructuring improves code organization.The package name change from
http.api.rpctonetwork.apiprovides a cleaner and more focused organization of network-related code.
20-26: LGTM! Imports reflect improved modularity.The imports demonstrate good practices:
- Using Fuel's authentication extension for proper token handling
- Centralizing network configuration through the Network module
- Maintaining consistent package structure for models
common/src/main/kotlin/com/lambda/network/api/v1/endpoints/LinkDiscord.kt (1)
20-27: LGTM! Well-organized imports.The imports are properly organized and include all necessary dependencies for HTTP requests, authentication, and network operations.
common/src/main/kotlin/com/lambda/network/api/v1/endpoints/CreateParty.kt (1)
18-26: LGTM! Well-organized imports and clear package structure.The imports are specific and the package structure follows a good versioned API pattern.
common/src/main/kotlin/com/lambda/network/api/v1/endpoints/GetParty.kt (2)
18-18: No issues found on the package declaration.
21-26: Imports look consistent.
The added imports for Fuel authentication and references to theNetworkmodule align well with the new approach of retrieving the API URL, version, and access token from a central place.common/src/main/kotlin/com/lambda/Lambda.kt (5)
20-23: No issues identified with these new imports.
34-34: No issues identified with the concurrent import.
46-49: No issues identified with the SSE-related imports.
51-51: No issues identified with theCountDownLatchimport.
65-65: No issues identified withisDebugproperty.common/src/main/kotlin/com/lambda/network/api/v1/endpoints/JoinParty.kt (1)
1-19: LGTM! Well-structured package with proper licensing.The file follows best practices with a clear GPL v3 license header and proper package structure.
common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (2)
57-57: Warn about empty token fallback.
Returning an empty token string can unintentionally produce authorization calls without valid credentials. Ensure that the call sites handle such scenarios or fail fast to avoid silent authentication failures.
66-74: Ensure concurrency safety during encryption events.
serverIdandhashare set in separate event listeners, and any race condition could lead to one being uninitialized by the time the other is used. Consider verifying thatEncryptionResponseis guaranteed to fire after theEncryptionRequest, or introduce a synchronization mechanism if necessary.common/src/main/kotlin/com/lambda/network/api/v1/endpoints/DeleteParty.kt (1)
29-33: Evaluate error handling to avoid silent failures.
The function returns a deserializedPartyor an error under.third, but no further handling is performed. If the deletion fails, a caller might not realize it unless it explicitly checks the result. Consider wrapping the response in a more robust error-handling mechanism or returning a sealed type to enforce usage checks.common/src/main/kotlin/com/lambda/util/math/MathUtils.kt (1)
41-41: LGTM! Clean and concise implementation.The
logCapfunction is well-placed in the MathUtils object and follows a functional style consistent with the rest of the file.common/src/main/kotlin/com/lambda/command/commands/DiscordCommand.kt (1)
67-81: Consider consistent execution strategy.The
acceptandrefusecommands userunConcurrentwhile other commands userunSafe. Consider using a consistent execution strategy unless there's a specific reason for the difference.Could you clarify why these commands require concurrent execution while others use safe execution?
common/src/main/resources/lambda.accesswidener (1)
71-73: LGTM! Well-structured additions.The new Vec3i setter methods are properly categorized and follow the file's conventions.
common/src/main/kotlin/com/lambda/event/listener/UnsafeListener.kt (1)
144-165: LGTM! Improved type safety and simplified implementation.The changes improve the function by:
- Making it more type-safe with a nullable return type.
- Simplifying the implementation using
Pointer.forge/build.gradle.kts (1)
38-38: LGTM! Simplified property assignment syntax.The change uses a more concise syntax for setting the
accessWidenerPathproperty while maintaining the same functionality.build.gradle.kts (2)
153-153: LGTM! Simplified compiler options syntax.The change uses a more concise syntax for setting the
jvmTargetproperty while maintaining the same functionality.
88-92:Details
❓ Verification inconclusive
Verify the purpose of the development property.
The added property
lambda.devwith a YouTube URL seems unusual. Please clarify its purpose and whether it's intended for development or debugging.
🌐 Web query:
What is the significance of the YouTube URL "youtu.be/RYnFIRc0k6E" in development configurations?💡 Result:
The YouTube URL
youtu.be/RYnFIRc0k6Erepresents a shortened link format used for sharing videos or configuring embedded players in development environments. Here’s its significance:1. URL Shortening for Simplicity
- The
youtu.bedomain acts as a built-in URL shortener for YouTube videos[6][9]. This makes links easier to share in contexts like social media or SMS[3].- Example:
youtu.be/RYnFIRc0k6Eredirects to the full video URL (e.g.,youtube.com/watch?v=RYnFIRc0k6E).2. Video ID Parameter
- The alphanumeric string
RYnFIRc0k6Eis the unique video ID, critical for identifying content programmatically[5][6]. Developers use this ID to:
- Embed videos via APIs[5].
- Track analytics or integrate with third-party tools[5][7].
3. Dynamic Configuration via Parameters
- Developers append query parameters to modify player behavior:
- Example:
youtu.be/RYnFIRc0k6E?autoplay=1&mute=1starts playback automatically with muted audio[5].- Common parameters include:
t=25: Start video at 25 seconds[4][7].rel=0: Disable related videos post-playback (deprecated in 2018)[5].origin=https://example.com: Track embedding source for analytics[5].4. Deprecated Features & Changes
- Post-2018 updates deprecated parameters like
showinfo, which previously hid video titles/uploader info[5].- The
relparameter now defaults to showing same-channel videos instead of disabling related content entirely[5].Summary
The
youtu.beURL simplifies sharing while enabling developers to customize video playback and tracking through query parameters. Its structure (video ID+ modifiers) remains essential for integrations requiring precise control over embedded content[5][7].Citations:
- 1: https://www.youtube.com/watch?v=IXJ5QsbW9R8
- 2: https://reasonsbehinddirectedenergyweapons.yourwebsitespace.com
- 3: https://seodesignchicago.com/marketing/how-to-change-your-youtube-url/
- 4: https://thebarchive.com/b/post/918514669/
- 5: https://developers.google.com/youtube/player_parameters
- 6: https://support.google.com/youtube/answer/6180214?hl=en
- 7: https://www.youtube.com/watch?v=8B5PmtjSzJc
- 8: https://support.google.com/youtube/answer/11585688?hl=en&co=GENIE.Platform%3DAndroid
- 9: https://www.youtube.com/watch?v=iUzyy8CnPVY
Clarify the
lambda.devProperty Usage in Development Configurations
- The configuration sets
lambda.devto a YouTube shortened URL (youtu.be/RYnFIRc0k6E), which is unconventional for development properties.- Please confirm whether this URL is intended for debugging, demonstration, or some specific runtime integration (e.g., embedding video support or a visual marker) in the development environment.
- If it’s a temporary placeholder or part of experimental instrumentation, adding inline documentation or a comment explaining its purpose would be beneficial.
| Fuel.post("${apiUrl}/api/${apiVersion.value}/link/discord") | ||
| .jsonBody("""{ "token": "$discordToken" }""") | ||
| .authentication() | ||
| .bearer(Network.accessToken) | ||
| .responseObject<Authentication>().third |
There was a problem hiding this comment.
Improve security and error handling.
The current implementation has several areas for improvement:
- Direct string interpolation in JSON body could be vulnerable to injection
- No error handling for failed requests
- URL construction could be more robust
Consider applying these improvements:
- Fuel.post("${apiUrl}/api/${apiVersion.value}/link/discord")
- .jsonBody("""{ "token": "$discordToken" }""")
- .authentication()
- .bearer(Network.accessToken)
- .responseObject<Authentication>().third
+ try {
+ val endpoint = "$apiUrl/api/${apiVersion.value}/link/discord"
+ val jsonBody = mapOf("token" to discordToken)
+
+ Fuel.post(endpoint)
+ .jsonBody(jsonBody)
+ .authentication()
+ .bearer(Network.accessToken)
+ .responseObject<Authentication>()
+ .third
+ .fold(
+ success = { it },
+ failure = { throw it }
+ )
+ } catch (e: Exception) {
+ throw IllegalStateException("Failed to link Discord account: ${e.message}", e)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fuel.post("${apiUrl}/api/${apiVersion.value}/link/discord") | |
| .jsonBody("""{ "token": "$discordToken" }""") | |
| .authentication() | |
| .bearer(Network.accessToken) | |
| .responseObject<Authentication>().third | |
| try { | |
| val endpoint = "$apiUrl/api/${apiVersion.value}/link/discord" | |
| val jsonBody = mapOf("token" to discordToken) | |
| Fuel.post(endpoint) | |
| .jsonBody(jsonBody) | |
| .authentication() | |
| .bearer(Network.accessToken) | |
| .responseObject<Authentication>() | |
| .third | |
| .fold( | |
| success = { it }, | |
| failure = { throw it } | |
| ) | |
| } catch (e: Exception) { | |
| throw IllegalStateException("Failed to link Discord account: ${e.message}", e) | |
| } |
| fun createParty() = | ||
| Fuel.post("${apiUrl}/api/${apiVersion.value}/party/create") | ||
| .authentication() | ||
| .bearer(Network.accessToken) | ||
| .responseObject<Party>().third |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for network failures and token validation.
While the implementation is concise and follows good practices for authentication, consider adding:
- Validation of
Network.accessTokenbefore use - Error handling for network failures
- Response status code validation
Here's a suggested implementation:
fun createParty() =
+ if (Network.accessToken.isBlank()) {
+ Result.failure<Party>(IllegalStateException("Access token not available"))
+ } else try {
Fuel.post("${apiUrl}/api/${apiVersion.value}/party/create")
.authentication()
.bearer(Network.accessToken)
- .responseObject<Party>().third
+ .responseObject<Party>()
+ .third
+ .fold(
+ { Result.success(it) },
+ { Result.failure(it) }
+ )
+ } catch (e: Exception) {
+ Result.failure(e)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun createParty() = | |
| Fuel.post("${apiUrl}/api/${apiVersion.value}/party/create") | |
| .authentication() | |
| .bearer(Network.accessToken) | |
| .responseObject<Party>().third | |
| fun createParty() = | |
| if (Network.accessToken.isBlank()) { | |
| Result.failure<Party>(IllegalStateException("Access token not available")) | |
| } else try { | |
| Fuel.post("${apiUrl}/api/${apiVersion.value}/party/create") | |
| .authentication() | |
| .bearer(Network.accessToken) | |
| .responseObject<Party>() | |
| .third | |
| .fold( | |
| { Result.success(it) }, | |
| { Result.failure(it) } | |
| ) | |
| } catch (e: Exception) { | |
| Result.failure(e) | |
| } |
| fun joinParty( | ||
| // The ID of the party. | ||
| // example: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" | ||
| partyId: String, | ||
| ) = |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for partyId.
The function should validate that the partyId is not empty and matches the expected format before making the request.
fun joinParty(
// The ID of the party.
// example: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6"
partyId: String,
-) =
+) {
+ require(partyId.isNotBlank()) { "Party ID cannot be empty" }
+ require(partyId.matches("[a-zA-Z0-9]+".toRegex())) { "Invalid party ID format" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun joinParty( | |
| // The ID of the party. | |
| // example: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" | |
| partyId: String, | |
| ) = | |
| fun joinParty( | |
| // The ID of the party. | |
| // example: "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6" | |
| partyId: String, | |
| ) { | |
| require(partyId.isNotBlank()) { "Party ID cannot be empty" } | |
| require(partyId.matches("[a-zA-Z0-9]+".toRegex())) { "Invalid party ID format" } | |
| // ... rest of the function implementation ... | |
| } |
| private suspend fun start() { | ||
| if (rpc.connected) return | ||
|
|
||
| runConcurrent { rpc.connect() } // TODO: Create a function that will wait until x seconds has passed or if the connection is successful | ||
| delay(1000) | ||
|
|
||
| val auth = rpc.applicationManager.authenticate() | ||
| val (authResp, error) = linkDiscord(discordToken = auth.accessToken) | ||
| if (error != null) { | ||
| warn(error.message.toString()) | ||
| return toast("Failed to link the discord account to the minecraft auth") | ||
| } | ||
|
|
||
| updateToken(authResp) | ||
| discordAuth = auth | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle or timeout the Discord connection attempt.
rpc.connect() is invoked, but if Discord is not installed or the user is offline, the method may hang or fail silently. Incorporate a timeout or additional error handling to provide user feedback in case of a timeout or failure.
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt
Outdated
Show resolved
Hide resolved
| Fuel.post("${apiUrl}/api/${apiVersion.value}/login") | ||
| .jsonBody("""{ "username": "$username", "hash": "$hash" }""") | ||
| .responseObject<Authentication>().third |
There was a problem hiding this comment.
Consider using a proper JSON serialization library.
Using string interpolation for JSON construction is prone to errors and security issues (e.g., if username contains quotes). Consider using Gson or Kotlinx.Serialization.
- .jsonBody("""{ "username": "$username", "hash": "$hash" }""")
+ .jsonBody(gson.toJson(mapOf(
+ "username" to username,
+ "hash" to hash
+ )))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Fuel.post("${apiUrl}/api/${apiVersion.value}/login") | |
| .jsonBody("""{ "username": "$username", "hash": "$hash" }""") | |
| .responseObject<Authentication>().third | |
| Fuel.post("${apiUrl}/api/${apiVersion.value}/login") | |
| .jsonBody(gson.toJson(mapOf( | |
| "username" to username, | |
| "hash" to hash | |
| ))) | |
| .responseObject<Authentication>().third |
| fun NbtCompound.getVector(key: String): Vec3i { | ||
| // TODO: Handle other cases like array of values, capitalized keys, etc | ||
| val compound = getCompound(key) | ||
| val x = compound.getInt("x") | ||
| val y = compound.getInt("y") | ||
| val z = compound.getInt("z") | ||
|
|
||
| return Vec3i(x, y, z) | ||
| } |
There was a problem hiding this comment.
Add error handling for missing or invalid NBT data.
The current implementation assumes the compound and its fields exist, which could lead to NullPointerException. Consider:
- Validating the compound exists
- Providing default values or handling missing coordinates
- Adding validation for coordinate values
fun NbtCompound.getVector(key: String): Vec3i {
// TODO: Handle other cases like array of values, capitalized keys, etc
- val compound = getCompound(key)
- val x = compound.getInt("x")
- val y = compound.getInt("y")
- val z = compound.getInt("z")
+ val compound = getCompound(key) ?: throw IllegalArgumentException("Vector compound not found for key: $key")
+ return Vec3i(
+ compound.getInt("x").takeIf { contains("x") } ?: 0,
+ compound.getInt("y").takeIf { contains("y") } ?: 0,
+ compound.getInt("z").takeIf { contains("z") } ?: 0
+ )
- return Vec3i(x, y, z)
}Committable suggestion skipped: line range outside the PR's diff.
…d.kt Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (1)
147-147: 🛠️ Refactor suggestionImplement a timeout or error handling for
rpc.connect().A
TODOis present to handle scenarios where Discord may not be installed or the user is offline. Without a timeout, the call might hang indefinitely.Would you like me to provide an implementation that tracks elapsed time and cancels if
rpc.connect()fails within a given limit?
🧹 Nitpick comments (2)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (2)
83-98: Possible concurrency risk with repeated coroutines.Both
listenConcurrently<WorldEvent.Join>(line 89) andonEnable { runConcurrent { … handleLoop() } }(line 96) can triggerhandleLoop(). This may spawn multiple loops updating the same shared state (currentParty,rpc) simultaneously, risking data races or unexpected behavior.Consider adding a guard to ensure that only one loop runs at a time or refactor the logic so that repeated calls won't conflict.
193-193: Revisit forced party exit on RPC disconnection.Line 193 forces a party leave when
rpc.connectedbecomes false. Users who briefly lose Discord connectivity might unintentionally lose their party.Allow a grace period or reattempt connection before forcing a party exit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Lambda
| fun SafeContext.partyCreate() { | ||
| if (!isDiscordLinked) return warn("You did not link your discord account") | ||
| if (player.isInParty) { | ||
| if (player.isPartyOwner) deleteParty() else leaveParty() | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Extra curly brace causing a syntax error.
The extra brace on line 109 disrupts the block structure for partyCreate() and prevents successful compilation.
Apply this fix:
- fun SafeContext.partyCreate() {
- if (!isDiscordLinked) return warn("You did not link your discord account")
- if (player.isInParty) {
- if (player.isPartyOwner) deleteParty() else leaveParty()
- return
- }
- }
+ fun SafeContext.partyCreate() {
+ if (!isDiscordLinked) return warn("You did not link your discord account")
+ if (player.isInParty) {
+ if (player.isPartyOwner) deleteParty() else leaveParty()
+ return
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fun SafeContext.partyCreate() { | |
| if (!isDiscordLinked) return warn("You did not link your discord account") | |
| if (player.isInParty) { | |
| if (player.isPartyOwner) deleteParty() else leaveParty() | |
| return | |
| } | |
| } | |
| fun SafeContext.partyCreate() { | |
| if (!isDiscordLinked) return warn("You did not link your discord account") | |
| if (player.isInParty) { | |
| if (player.isPartyOwner) deleteParty() else leaveParty() | |
| return | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (1)
70-70: Consider scoping the RPC client
TheKDiscordIPCinstance is defined at module level, yet its lifecycle depends on the module’s enable/disable state. In a multi-module system, consider scoping or re-initializingrpcmore granularly to avoid potential leaks when toggling the module on/off.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
common/src/main/kotlin/com/lambda/command/commands/DiscordCommand.kt(1 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/main/kotlin/com/lambda/command/commands/DiscordCommand.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Lambda
🔇 Additional comments (3)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (3)
157-172: [Duplicate Comment] Handle or timeout the Discord connection attempt
A previous review mentioned adding a timeout or additional error handling if Discord is unavailable or not installed. Currently,rpc.connect()may hang. Implement a timeout to provide user feedback and prevent blocking the coroutine indefinitely.
195-207: Verify forced party leave upon Discord disconnection
Whenrpc.connectedis false, the code callsleaveParty()ifisDiscordLinked. This can abruptly remove a user from an active party if Discord times out or disconnects briefly. Verify that this is the intended behavior and not an unintended side effect.
89-94: Ensure concurrency safety in event listeners
Creating or joining a party from multiple listeners (e.g., onWorldEvent.Join, then inonEnable) might introduce race conditions or duplicate calls if the game condition triggers both. Verify that your concurrency model prevents simultaneous calls topartyCreate().
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (3)
59-59: Remove or rephrase unprofessional comment.The comment “gson is too stupid” is unprofessional. Please update it to describe the technical limitation or reason more clearly.
80-88: Handle API authentication failures more robustly.Currently, upon authentication failure, the code logs an error and returns
false, but no fallback or retry is performed. Consider adding more robust error handling to inform users (e.g., in-game notification) or retry logic if necessary.
100-100: Use a more descriptive variable name.The local variable
adddddis unclear and could lead to confusion. Rename it to something more descriptive, likeparsedServerAddress.- val addddd = ServerAddress.parse(authServer) + val parsedServerAddress = ServerAddress.parse(authServer)common/src/main/kotlin/com/lambda/Lambda.kt (2)
61-61: Ensure debug flag usage is documented.
isDebugis inferred from system properties, which may lead to confusion for users or team members. Document this flag’s purpose or provide a clearer mechanism for toggling debug mode (e.g., environment variable or a config file).
92-110: Use consistent logging instead of println.Currently, SSE events are printed via
println. In production or debug builds, it’s often preferable to use a logger (Lambda.LOGor similar) so logs can be properly categorized and filtered.common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (2)
195-207: Revisit forcing a party leave upon disconnection from Discord.
InhandleLoop(), oncerpc.connectedbecomes false, you callleaveParty()ifisDiscordLinked. This might surprise users who want to remain in their party. Consider prompting or allowing them to stay in the party without active Discord Rich Presence.
209-228: Refactor the activity-building logic for maintainability.
The inline assembly of Discord activity details (lines 212-227) is functional, but centralizing repeated string concatenations and party logic into helper functions or a small builder class could boost clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
common/src/main/kotlin/com/lambda/Lambda.kt(4 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Network.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Authentication.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/main/kotlin/com/lambda/network/api/v1/models/Authentication.kt
🔇 Additional comments (5)
common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (1)
90-92: Validate JWT decoding.When decoding the base64-encoded token payload, an invalid or malformed JWT could cause runtime errors. Consider handling potential parsing exceptions and validating the token structure before deserializing.
common/src/main/kotlin/com/lambda/Lambda.kt (1)
79-85: Consider error handling for unavailable SSE endpoint.If
http://localhost:8080/api/v1/party/listenis unreachable, the current code flow might throw an exception in the future or hang. Consider implementing retry logic, fallback behavior, or user notification if the SSE endpoint is offline or returns an error.common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (3)
51-66: Looks good!
The module initialization with default settings (e.g.,Page, presence lines, etc.) appears consistent. The separation of "General" and "Party" settings is well-structured, and the default ranges fordelayseem appropriate.
157-161: Consider adding a timeout or fallback forrpc.connect().
Becauserpc.connect()is called without handling the possibility of Discord not being installed or user being offline, the method may hang/trap. This has been mentioned in a past review comment.A potential improvement would be to wrap
rpc.connect()in a function that times out or gracefully handles failure:withTimeoutOrNull(5000) { rpc.connect() } ?: warn("Discord connection timed out")
234-249: Guard against null or invalid context references inLineInfo.
Although unlikely in standard usage, calls such asmc.player?.healthormc.currentServerEntrycould becomenullin uninitialized states or edge cases (e.g., reinitialization or dimension transitions). Ensure the code gracefully handles these corner cases.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (1)
74-80:⚠️ Potential issueEnsure thread-safe hash computation.
The hash computation lacks proper synchronization which could lead to race conditions.
listenUnsafe<EncryptionResponse> { event -> if (event.secretKey.isDestroyed) return@listenUnsafe - - hash = BigInteger( - NetworkEncryptionUtils.computeServerId(serverId, event.publicKey, event.secretKey) - ).toString(16) + synchronized(this) { + hash = BigInteger( + NetworkEncryptionUtils.computeServerId(serverId, event.publicKey, event.secretKey) + ).toString(16) + } }
🧹 Nitpick comments (7)
common/src/main/kotlin/com/lambda/network/api/v1/models/Party.kt (1)
26-26: Fix indentation: Replace tabs with spaces.The
@SerializedNameannotations are using tabs for indentation while the rest of the file uses spaces. For consistency, replace tabs with spaces.- @SerializedName("id") + @SerializedName("id") - @SerializedName("join_secret") + @SerializedName("join_secret") - @SerializedName("leader") + @SerializedName("leader") - @SerializedName("creation") + @SerializedName("creation") - @SerializedName("players") + @SerializedName("players")Also applies to: 31-31, 35-35, 40-40, 44-44
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (1)
106-117: Revisit forcing party deletion when already in a party.
CallingdeletePartyorleavePartyunconditionally if the player is in a party might surprise users who want to remain in their existing party. Consider prompting the user or providing a separate command for forcibly recreating a party.common/src/main/kotlin/com/lambda/module/modules/client/Network.kt (5)
2-2: Update the copyright year.The copyright year is set to 2025, which is in the future. Update it to the current year or the year when the code was written.
-* Copyright 2025 Lambda +* Copyright 2024 Lambda
52-52: Add a meaningful module description.The current description is a placeholder ("..."). Add a proper description that explains the module's purpose and functionality.
- description = "...", + description = "Handles network authentication and API communication for Lambda client",
60-61: Improve state management and documentation.Several issues to address:
- The TODO comment about caching should be implemented or tracked in an issue
- The comment about gson being "stupid" is unprofessional
- Consider adding thread safety annotations for mutable state variables
- var apiAuth: Authentication? = null; private set // TODO: Cache - var deserialized: Data? = null; private set // gson is too stupid + @Volatile + var apiAuth: Authentication? = null; private set + @Volatile + var deserialized: Data? = null; private set // Deserialized JWT payloadWould you like me to create an issue to track the caching implementation?
129-129: Add documentation for token management.The
updateTokenfunction lacks documentation explaining its purpose and usage.+ /** + * Updates the authentication token for the API client. + * This is an internal function used during the authentication flow. + * + * @param auth The new authentication data, or null to clear the current authentication + */ internal fun updateToken(auth: Authentication?) { apiAuth = auth }
131-134: Enhance API version management.Consider adding version metadata to improve API version management.
- enum class ApiVersion(val value: String) { + enum class ApiVersion( + val value: String, + val isDeprecated: Boolean = false, + val deprecatedSince: String? = null + ) { // We can use @Deprecated("Not supported") to remove old API versions in the future - V1("v1"), + V1("v1", isDeprecated = false), }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)common/src/main/kotlin/com/lambda/module/modules/client/Network.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/endpoints/PartyUpdates.kt(1 hunks)common/src/main/kotlin/com/lambda/network/api/v1/models/Party.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- common/src/main/kotlin/com/lambda/network/api/v1/endpoints/PartyUpdates.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Lambda
🔇 Additional comments (7)
common/src/main/kotlin/com/lambda/network/api/v1/models/Party.kt (3)
18-18: LGTM! Package name aligns with network module introduction.The package name change from
http.apitonetwork.apibetter reflects the new network module architecture described in the PR objectives.
20-21: LGTM! Imports are clean and necessary.The imports are minimal and appropriate for the class requirements.
23-46: LGTM! Well-structured data class with clear documentation.The data class is well-documented with clear comments explaining each property's purpose. The removal of the
settingsproperty aligns with the model simplification mentioned in the summary.common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (4)
83-101: Consider potential concurrency overlap in listener callbacks.
When bothlistenConcurrently<WorldEvent.Join>and the module'sonEnableblock callstart()andhandleLoop(), multiple coroutines might be launched if the module is toggled quickly. Although you guard re-initialization withif (rpc.connected) returninstart(), confirm that no overlapping coroutines alter shared state unpredictably.
159-174: Handle or timeout the Discord connection attempt.
rpc.connect()might hang if Discord is not installed or offline. Add a timeout or improved error handling to give user feedback on failure.
197-209: Verify logic for forced party leave on RPC disconnection.
When the loop ends, the code callsleaveParty()ifisDiscordLinkedis still true. This may remove the user from a party even if they temporarily lost Discord connectivity. Confirm this aligns with user experience goals, or handle the party session more gracefully.
219-220: Check user identity exposure in smallImage URL.
The code references an external service (mc-heads.net) using the player's unique ID and username. This typically poses minimal risk, but verify that sharing these identifiers is acceptable from a privacy standpoint.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
common/src/main/kotlin/com/lambda/util/extension/World.kt (1)
30-30: Improve readability and add documentation.While the logic is correct, the single-line implementation reduces readability. Consider expanding the when expression and adding documentation to explain the property's purpose, especially since it's used for network and Discord-related features.
Apply this diff to improve readability:
+/** + * Returns the current game context name. + * Used for network status and Discord Rich Presence. + */ -val SafeContext.worldName: String get() = when { mc.currentServerEntry != null -> "Multiplayer"; mc.isIntegratedServerRunning -> "Singleplayer"; else -> "Main Menu" } +val SafeContext.worldName: String + get() = when { + mc.currentServerEntry != null -> "Multiplayer" + mc.isIntegratedServerRunning -> "Singleplayer" + else -> "Main Menu" + }common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (2)
43-43: Consider making the module enabled by default or removing the commented-out code.
Currently,"enabledByDefault = true"is commented out. If you plan to ship the feature, consider enabling it or removing the comment to avoid confusion.
58-74: Unify initialization paths to avoid duplicating logic.
The code inlistenOnce<WorldEvent.Join>and in theonEnablecallback both initiaterunConcurrent { start(); handleLoop() }. Consider consolidating them into a single entry point to reduce duplication and make the flow more predictable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt(1 hunks)common/src/main/kotlin/com/lambda/util/extension/World.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build Lambda
🔇 Additional comments (3)
common/src/main/kotlin/com/lambda/module/modules/client/Discord.kt (3)
79-80: Apply a timeout or additional error handling when connecting to Discord.
rpc.connect()might hang or fail silently if Discord is unavailable. This repeats a previously noted concern about adding a timeout or error-handling callback to notify users.
95-99: Verify the logic to exit the update loop.
While this loop checksrpc.connected, ensure there's a straightforward exit path in scenarios where the socket never disconnects. Otherwise, you risk an indefinite loop that could hold resources.
107-107: Confirm that sharing player UUID with a third-party avatar service complies with privacy requirements.
Using themc.gameProfile.idto fetch avatars from “mc-heads.net” may expose personally identifiable information. Confirm that this approach is acceptable.
This pull request introduces a Network module which handles basic authentication against the lambda api
Additionally, players can link their discord account using the rpc to access party features
These features include:
The commit 3a3feba was accidentally pushed on the wrong branch due to github desktop bugs, it does not show up on my machine but does on remote
This pull request is not ready to be merged
Summary by CodeRabbit
New Features
Bug Fixes
Chores