Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a pluggable lobby-code encoding system (IBaseEncoder + Base36/Base62), a LobbyCode runtime utility for encoder discovery/assignment, editor UI to select encoder, and updates to lobby creation/join flows to encode/decode lobby codes. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as Editor/UI
participant LM as LobbyManager
participant LC as LobbyCode
participant ENC as Encoder
participant Provider as SteamProvider
Note over LM: Initialization
Editor->>LM: set lobbyCodeEncoderType (serialized)
LM->>LC: AssignEncoder(type)
LC->>ENC: instantiate selected encoder
ENC-->>LC: encoder instance stored
Note over Provider,LM: Lobby creation / reporting
Provider->>LM: Create/Report lobby (lobbyId)
LM->>LC: Encode(ulong lobbyId)
LC->>ENC: Encode(ulong)
ENC-->>LC: encoded string
LC-->>LM: return LobbyCode
LM->>Provider: store/send LobbyCode
Note over UI,LM: Join by code
Editor->>LM: JoinLobbyByCode(code)
LM->>LC: Decode(code)
LC->>ENC: Decode(string)
ENC-->>LC: ulong id
LC-->>LM: roomId
LM->>LM: JoinLobby(roomId)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Assets/PurrLobby/Runtime/Misc/UI/JoinButton.cs (1)
17-17:⚠️ Potential issue | 🟡 MinorStale warning message references "room ID" instead of "lobby code".
Since
JoinLobbyByCodenow expects a lobby code rather than a raw room ID, the log message should be updated to reflect the new terminology.📝 Proposed fix
- Debug.LogWarning($"Can't start join, room ID is empty."); + Debug.LogWarning($"Can't start join, lobby code is empty.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Misc/UI/JoinButton.cs` at line 17, Update the stale log message in JoinButton's join flow: wherever Debug.LogWarning($"Can't start join, room ID is empty."); is used (in the JoinLobbyByCode / OnJoinButtonPressed logic), change the wording to reference "lobby code" instead of "room ID" so the message accurately reflects the new parameter name (e.g., "Can't start join, lobby code is empty.").Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs (1)
308-333:⚠️ Potential issue | 🟠 Major
SearchLobbiesAsyncdoesn't populateLobbyCode, breaking join-by-code for search results.The
Lobbystructs returned fromSearchLobbiesAsynchave noLobbyCodeset (using a direct struct initializer without that field). Per the AI summary,LobbyEntrynow callsJoinLobbyByCode(_room.LobbyCode). WithLobbyCodebeing an empty string,LobbyCode.Decode("")throwsArgumentException("Value cannot be null or empty"), so users can never join a lobby found through search.The other lobby creation/join paths all use
LobbyFactory.Create(...)with an encoded code;SearchLobbiesAsyncshould do the same:🐛 Proposed fix
- results.Add(new Lobby - { - Name = Steamworks.SteamMatchmaking.GetLobbyData(lobbyId, "Name"), - IsValid = true, - LobbyId = lobbyId.m_SteamID.ToString(), - MaxPlayers = maxPlayers, - Properties = lobbyProperties, - Members = GetLobbyUsers(lobbyId) - }); + results.Add(LobbyFactory.Create( + Steamworks.SteamMatchmaking.GetLobbyData(lobbyId, "Name"), + lobbyId.m_SteamID.ToString(), + LobbyCode.Encode(lobbyId.m_SteamID), + maxPlayers, + false, + GetLobbyUsers(lobbyId), + lobbyProperties + ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs` around lines 308 - 333, SearchLobbiesAsync currently constructs Lobby structs directly and omits LobbyCode, causing join-by-code to fail; update the callback in SearchLobbiesAsync to create each result via LobbyFactory.Create(...) (or the same factory method used by other creation paths) so the returned Lobby objects include the encoded LobbyCode (use the same inputs you already fetch: lobbyId, Name, maxPlayers, Properties, Members) instead of the direct struct initializer. Ensure you reference the existing methods GetLobbyProperties, GetLobbyUsers and the Steamworks lobbyId when calling LobbyFactory.Create so LobbyCode is populated and compatible with LobbyEntry.JoinLobbyByCode.
♻️ Duplicate comments (1)
Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs (1)
113-121:⚠️ Potential issue | 🔴 Critical
LobbyCode.Encodewill throw aNullReferenceExceptionif no encoder has been assigned.
LobbyCode.encoderisnullby default. IflobbyCodeEncoderTypeis empty (the serialized field default),AssignEncodersilently no-ops andencoderstaysnull. Any call toLobbyCode.Encode(here and at lines 256, 575, 646) will then NPE at runtime. The root fix belongs inLobbyCode.cs/LobbyManager.cs— see comments there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs` around lines 113 - 121, LobbyCode.Encode is being called when LobbyCode.encoder can be null (because lobbyCodeEncoderType may be empty and AssignEncoder is a no-op), causing a NullReferenceException; update LobbyCode.Encode (and/or LobbyManager initialization) to null-check LobbyCode.encoder and either lazily assign a default encoder or throw a clear ArgumentException, and ensure AssignEncoder sets a fallback encoder when lobbyCodeEncoderType is empty; search for usages (LobbyCode.Encode, LobbyCode.encoder, AssignEncoder) and make Encode robust so calls from SteamLobbyProvider.Create (and other sites at lines noted) cannot NPE at runtime.
🧹 Nitpick comments (3)
Assets/PurrLobby/Runtime/Lobby/Lobby.cs (1)
9-9: Consider renaming theLobbyCodefield to avoid shadowing theLobbyCodestatic class.
Lobby.LobbyCode(string field) andLobbyCode(static class) share the same unqualified name in thePurrLobbynamespace. The object-initializer inLobbyFactory.Createresolves the right-handLobbyCode.Encode(…)correctly to the static class, but the collision creates readability friction for future maintainers. A name likeEncodedCodeorJoinCodewould be unambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Lobby/Lobby.cs` at line 9, The instance field named LobbyCode in class Lobby conflicts with the static LobbyCode class and creates ambiguity; rename the instance field (e.g., EncodedCode or JoinCode) and update all references, including the object-initializer in LobbyFactory.Create where LobbyCode.Encode(...) is used, so that the field no longer shadows the static class and the code reads unambiguously.Assets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.cs (2)
25-43:Decodedoesn't trim whitespace — inconsistent withBase36Encoder.Decode.
Base36Encoder.Decodenormalises input with.Trim()before iterating characters.Base62Encoder.Decodedoesn't, so a lobby code with a leading/trailing space will throwFormatExceptioninstead of being silently accepted. Since Base62 is case-sensitive,.ToUpper()should not be applied, but.Trim()is safe to add for parity.♻️ Proposed fix
+ value = value.Trim(); ulong result = 0; foreach (var c in value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.cs` around lines 25 - 43, The Decode method in Base62Encoder (Decode) should normalize input by trimming whitespace before processing; update the method to call value = value.Trim() (or use a trimmed local variable) at the start after the null/empty check so leading/trailing spaces don't cause FormatException, and do not apply ToUpper() so case-sensitivity is preserved.
16-22: Avoid O(n²) string prepend inEncode; build with achararray orStringBuilderinstead.
result = Chars[…] + resultcreates a new string allocation on every loop iteration. For Base62, aulongmax value produces at most 11 characters, so the practical impact is negligible — but for consistency with the codebase's Base36Encoder pattern, consider using achar[]reverse-fill:♻️ Proposed refactor
- var result = ""; - while (value > 0) - { - result = Chars[(int)(value % 62)] + result; - value /= 62; - } - return result; + var buffer = new char[11]; // ulong max in base-62 is 11 chars + int pos = buffer.Length; + while (value > 0) + { + buffer[--pos] = Chars[(int)(value % 62)]; + value /= 62; + } + return new string(buffer, pos, buffer.Length - pos);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.cs` around lines 16 - 22, The Encode method in Base62Encoder currently prepends to a string in the loop (result = Chars[...] + result) causing repeated allocations; change it to build the output in reverse using a fixed char[] (max length 11 for ulong) and then return a new string from that slice (or use a StringBuilder and Append then Reverse), mirroring the pattern used in Base36Encoder; update references in the Encode method and use the Chars lookup to fill the char buffer from the end towards the start, then construct the final string once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Assets/PurrLobby/Editor/LobbyManagerEditor.cs`:
- Around line 90-102: In DrawEncoderDropdown: call serializedObject.Update()
before reading the "lobbyCodeEncoderType" property, and guard against empty
encoderNames (returned by GetEncoderTypes) before calling EditorGUILayout.Popup;
if encoderNames.Length == 0, show a disabled label/HelpBox or early-return to
avoid calling Popup with no options. After Popup, validate encoderSelectedIndex
is within bounds (0 <= index < encoderNames.Length) before assigning
property.stringValue = encoderNames[encoderSelectedIndex], and only call
serializedObject.ApplyModifiedProperties() when a valid change occurred.
In `@Assets/PurrLobby/Runtime/Lobby/Lobby.cs`:
- Line 43: LobbyCode.Encode can NRE if its static field IBaseEncoder encoder is
null; modify LobbyCode so encoder defaults to a concrete implementation (e.g.,
set encoder = new Base62Encoder()) and update AssignEncoder to log a warning or
throw a descriptive exception when the requested encoder type
(lobbyCodeEncoderType) is not found instead of silently returning; ensure
LobbyCode.Encode, AssignEncoder and the IBaseEncoder field are updated so
LobbyFactory.Create or any caller (before LobbyManager.Awake) always has a valid
encoder or gets a clear diagnostic message.
- Line 43: The LobbyCode assignment uses uint.Parse on a Steam CSteamID string
which can overflow; change the parsing to use ulong.Parse so LobbyCode =
LobbyCode.Encode(ulong.Parse(lobbyId)) instead of uint.Parse; update the call
site referencing LobbyCode.Encode and the lobbyId variable to parse as ulong to
match IBaseEncoder.Encode's expected type.
In `@Assets/PurrLobby/Runtime/Lobby/LobbyCode.cs`:
- Around line 19-34: GetEncoderTypes should guard against Assembly.GetTypes
throwing ReflectionTypeLoadException by catching that exception, extracting
non-null types from ex.Types, and continuing (so GetEncoderTypes returns a
best-effort list); in AssignEncoder, when types.FirstOrDefault(t => t.Name ==
name) yields null, do not fail silently—emit a Debug.LogWarning (include the
requested name like lobbyCodeEncoderType and optionally available encoder type
names) so misconfiguration is visible, and only call Activator.CreateInstance
and assign to encoder when the resolved type is non-null.
- Around line 9-17: The static field 'encoder' (IBaseEncoder) is null by default
causing NREs in LobbyCode.Encode/Decode when AssignEncoder wasn't called; fix by
ensuring a safe default or a clear guard: initialize 'encoder' to a sensible
default implementation (e.g., a new DefaultLobbyCodeEncoder or
SimpleBaseEncoder) when declared or in a static constructor so Encode/Decode
always have a valid instance, and update AssignEncoder to replace it if a custom
type is provided (reference symbols: IBaseEncoder, encoder, Encode, Decode,
AssignEncoder, LobbyManager.lobbyCodeEncoderType); alternatively, add a
defensive null-check in Encode/Decode that throws an informative exception
mentioning LobbyManager.lobbyCodeEncoderType and suggests calling AssignEncoder
or configuring the LobbyManager.
In `@Assets/PurrLobby/Runtime/Lobby/LobbyManager.cs`:
- Line 14: The serialized field lobbyCodeEncoderType on LobbyManager defaults to
an empty string which causes LobbyCode.AssignEncoder("") to leave
LobbyCode.encoder null; update LobbyManager to provide a valid default encoder
string (or detect empty and substitute the default encoder type name) and call
LobbyCode.AssignEncoder(defaultTypeName) during initialization (e.g.,
Awake/Start) so LobbyCode.encoder is always assigned before any calls to
LobbyCode.Encode/Decode; reference lobbyCodeEncoderType,
LobbyCode.AssignEncoder, LobbyCode.encoder, and LobbyCode.Encode/Decode when
making the change.
- Around line 279-283: JoinLobbyByCode currently calls LobbyCode.Decode
synchronously which can throw ArgumentException or FormatException and crash the
caller; update JoinLobbyByCode to mirror JoinLobby's validation by wrapping the
LobbyCode.Decode call in a try/catch that catches ArgumentException and
FormatException, calls OnRoomJoinFailed with a clear reason (and returns)
instead of letting exceptions propagate, and only calls
JoinLobby(roomId.ToString()) when decode succeeds; reference: JoinLobbyByCode,
LobbyCode.Decode, OnRoomJoinFailed, JoinLobby (and caller JoinButton.JoinRoom)
to locate the relevant code paths.
In `@Assets/PurrLobby/Runtime/Misc/Encoders/Base36Encoder.cs`:
- Around line 33-43: The loop that computes result = result * 36 + (ulong)digit
can silently overflow for Base36 strings longer than fit in ulong; update the
Base36 decode routine to prevent silent wrapping by enforcing a maximum input
length (e.g., if (value.Length > 13) throw new FormatException("Base36 string
too long for ulong")) and/or perform the multiplication/add inside a checked
block so an OverflowException is raised and translated to a clear
FormatException; reference the variable/result computation (result,
Chars.IndexOf(c), and the expression result = result * 36 + (ulong)digit) when
applying the length guard or wrapping the arithmetic in checked and rethrowing a
descriptive error.
In `@Assets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs`:
- Line 1: Remove the unused directive by deleting the top-line "using
UnityEngine;" from the IBaseEncoder.cs file; the interface declaration
(IBaseEncoder) does not reference any UnityEngine types so removing that using
statement cleans up dead code without affecting functionality or compilation.
---
Outside diff comments:
In `@Assets/PurrLobby/Runtime/Misc/UI/JoinButton.cs`:
- Line 17: Update the stale log message in JoinButton's join flow: wherever
Debug.LogWarning($"Can't start join, room ID is empty."); is used (in the
JoinLobbyByCode / OnJoinButtonPressed logic), change the wording to reference
"lobby code" instead of "room ID" so the message accurately reflects the new
parameter name (e.g., "Can't start join, lobby code is empty.").
In `@Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs`:
- Around line 308-333: SearchLobbiesAsync currently constructs Lobby structs
directly and omits LobbyCode, causing join-by-code to fail; update the callback
in SearchLobbiesAsync to create each result via LobbyFactory.Create(...) (or the
same factory method used by other creation paths) so the returned Lobby objects
include the encoded LobbyCode (use the same inputs you already fetch: lobbyId,
Name, maxPlayers, Properties, Members) instead of the direct struct initializer.
Ensure you reference the existing methods GetLobbyProperties, GetLobbyUsers and
the Steamworks lobbyId when calling LobbyFactory.Create so LobbyCode is
populated and compatible with LobbyEntry.JoinLobbyByCode.
---
Duplicate comments:
In `@Assets/PurrLobby/Runtime/Providers/SteamLobbyProvider.cs`:
- Around line 113-121: LobbyCode.Encode is being called when LobbyCode.encoder
can be null (because lobbyCodeEncoderType may be empty and AssignEncoder is a
no-op), causing a NullReferenceException; update LobbyCode.Encode (and/or
LobbyManager initialization) to null-check LobbyCode.encoder and either lazily
assign a default encoder or throw a clear ArgumentException, and ensure
AssignEncoder sets a fallback encoder when lobbyCodeEncoderType is empty; search
for usages (LobbyCode.Encode, LobbyCode.encoder, AssignEncoder) and make Encode
robust so calls from SteamLobbyProvider.Create (and other sites at lines noted)
cannot NPE at runtime.
---
Nitpick comments:
In `@Assets/PurrLobby/Runtime/Lobby/Lobby.cs`:
- Line 9: The instance field named LobbyCode in class Lobby conflicts with the
static LobbyCode class and creates ambiguity; rename the instance field (e.g.,
EncodedCode or JoinCode) and update all references, including the
object-initializer in LobbyFactory.Create where LobbyCode.Encode(...) is used,
so that the field no longer shadows the static class and the code reads
unambiguously.
In `@Assets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.cs`:
- Around line 25-43: The Decode method in Base62Encoder (Decode) should
normalize input by trimming whitespace before processing; update the method to
call value = value.Trim() (or use a trimmed local variable) at the start after
the null/empty check so leading/trailing spaces don't cause FormatException, and
do not apply ToUpper() so case-sensitivity is preserved.
- Around line 16-22: The Encode method in Base62Encoder currently prepends to a
string in the loop (result = Chars[...] + result) causing repeated allocations;
change it to build the output in reverse using a fixed char[] (max length 11 for
ulong) and then return a new string from that slice (or use a StringBuilder and
Append then Reverse), mirroring the pattern used in Base36Encoder; update
references in the Encode method and use the Chars lookup to fill the char buffer
from the end towards the start, then construct the final string once.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Assets/PurrLobby/LobbyScenes/LobbySample.unityis excluded by!**/*.unity
📒 Files selected for processing (16)
Assets/PurrLobby/Editor/LobbyManagerEditor.csAssets/PurrLobby/Runtime/Lobby/Lobby.csAssets/PurrLobby/Runtime/Lobby/LobbyCode.csAssets/PurrLobby/Runtime/Lobby/LobbyCode.cs.metaAssets/PurrLobby/Runtime/Lobby/LobbyManager.csAssets/PurrLobby/Runtime/Misc/Encoders.metaAssets/PurrLobby/Runtime/Misc/Encoders/Base36Encoder.csAssets/PurrLobby/Runtime/Misc/Encoders/Base36Encoder.cs.metaAssets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.csAssets/PurrLobby/Runtime/Misc/Encoders/Base62Encoder.cs.metaAssets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.csAssets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs.metaAssets/PurrLobby/Runtime/Misc/UI/JoinButton.csAssets/PurrLobby/Runtime/Misc/UI/LobbyEntry.csAssets/PurrLobby/Runtime/Providers/SteamLobbyProvider.csAssets/PurrLobby/Runtime/ViewManagement/Views/LobbyView.cs
Comment on lines
+279
to
+283
| public void JoinLobbyByCode(string lobbyCode) | ||
| { | ||
| var roomId = LobbyCode.Decode(lobbyCode); | ||
| JoinLobby(roomId.ToString()); | ||
| } |
There was a problem hiding this comment.
JoinLobbyByCode doesn't handle decode exceptions; invalid codes crash silently instead of emitting OnRoomJoinFailed.
LobbyCode.Decode throws ArgumentException for empty input and FormatException for invalid characters. Both are thrown synchronously before entering RunTask, so they propagate unhandled to the UI caller (e.g., JoinButton.JoinRoom). The method should mirror JoinLobby's existing validation pattern:
🐛 Proposed fix
public void JoinLobbyByCode(string lobbyCode)
{
- var roomId = LobbyCode.Decode(lobbyCode);
- JoinLobby(roomId.ToString());
+ if (string.IsNullOrEmpty(lobbyCode))
+ {
+ OnRoomJoinFailed?.Invoke("Lobby code is null or empty.");
+ return;
+ }
+ try
+ {
+ var roomId = LobbyCode.Decode(lobbyCode);
+ JoinLobby(roomId.ToString());
+ }
+ catch (Exception ex)
+ {
+ OnRoomJoinFailed?.Invoke($"Invalid lobby code: {ex.Message}");
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Assets/PurrLobby/Runtime/Lobby/LobbyManager.cs` around lines 279 - 283,
JoinLobbyByCode currently calls LobbyCode.Decode synchronously which can throw
ArgumentException or FormatException and crash the caller; update
JoinLobbyByCode to mirror JoinLobby's validation by wrapping the
LobbyCode.Decode call in a try/catch that catches ArgumentException and
FormatException, calls OnRoomJoinFailed with a clear reason (and returns)
instead of letting exceptions propagate, and only calls
JoinLobby(roomId.ToString()) when decode succeeds; reference: JoinLobbyByCode,
LobbyCode.Decode, OnRoomJoinFailed, JoinLobby (and caller JoinButton.JoinRoom)
to locate the relevant code paths.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Assets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs (2)
5-6:publicon interface members is redundant — consider removing.Interface members are public by default, and you can explicitly specify accessibility modifiers such as
public,protected,internal,private, etc. Omitting the explicitpublicis the idiomatic C# convention for abstract interface declarations and reduces visual noise.♻️ Proposed refactor
public interface IBaseEncoder { - public string Encode(ulong value); - public ulong Decode(string value); + string Encode(ulong value); + ulong Decode(string value); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs` around lines 5 - 6, Remove the redundant 'public' accessibility modifiers from the interface member declarations in IBaseEncoder (the Encode(string) and Decode(ulong) signatures); interface members are public by default in C#, so update the declarations to omit 'public' while keeping the method names Encode and Decode and their parameter/return types unchanged.
5-6: Consider documenting the exception contract.Both
Base36Encoder.DecodeandBase62Encoder.DecodethrowArgumentExceptionon null/empty input andFormatExceptionon invalid characters. Since the interface is the public surface implementors program against, callers have no documented expectation of what to catch. XML doc comments (/// <exception cref="...">) on the interface method signatures would surface this contract in IDEs and generated docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs` around lines 5 - 6, Add XML documentation on the IBaseEncoder interface methods to declare the exception contract callers should expect: add /// <exception cref="System.ArgumentException"> for Decode when input is null or empty and /// <exception cref="System.FormatException"> for Decode when input contains invalid characters (matching Base36Encoder.Decode and Base62Encoder.Decode behavior); update the Decode(string value) method's doc to include both exceptions and a short description, and add basic summary/exception docs for Encode/Decode so IDEs and generated docs surface the expected exceptions to implementors and callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Assets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs`:
- Around line 5-6: Remove the redundant 'public' accessibility modifiers from
the interface member declarations in IBaseEncoder (the Encode(string) and
Decode(ulong) signatures); interface members are public by default in C#, so
update the declarations to omit 'public' while keeping the method names Encode
and Decode and their parameter/return types unchanged.
- Around line 5-6: Add XML documentation on the IBaseEncoder interface methods
to declare the exception contract callers should expect: add /// <exception
cref="System.ArgumentException"> for Decode when input is null or empty and ///
<exception cref="System.FormatException"> for Decode when input contains invalid
characters (matching Base36Encoder.Decode and Base62Encoder.Decode behavior);
update the Decode(string value) method's doc to include both exceptions and a
short description, and add basic summary/exception docs for Encode/Decode so
IDEs and generated docs surface the expected exceptions to implementors and
callers.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Assets/PurrLobby/Runtime/Lobby/LobbyCode.csAssets/PurrLobby/Runtime/Misc/Encoders/IBaseEncoder.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- Assets/PurrLobby/Runtime/Lobby/LobbyCode.cs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Assets/PurrLobby/Editor/LobbyManagerEditor.cs`:
- Around line 22-26: OnEnable currently calls encoderTypes =
LobbyCode.GetEncoderTypes() and immediately does encoderNames =
encoderTypes.Select(...), which will NRE if GetEncoderTypes() returns null;
change it to guard against null by treating encoderTypes as an empty collection
when null (e.g., coalesce GetEncoderTypes() to Array.Empty<Type>() or similar)
before calling Select, and ensure encoderNames is assigned from that safe
collection so encoderNames is always a valid string[].
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
1 participant
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 CodeRabbit