| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Sorry, something went wrong.
WalkthroughRemoved the WalletConnect Unity integration and related assets (core, modal, nethereum, UI, native bridges and many .meta/docs), updated PlaygroundManager to use ReownWallet (changed chain ID and contract addresses, disconnect active wallet on init), and removed WalletConnect editor fields/UI; minor editor/UI sizing and formatting tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Playground as PlaygroundManager
participant SDK as Thirdweb SDK
participant Wallet as Reown Wallet
User->>Playground: Click "Connect"
Playground->>SDK: GetWalletOptions(WalletProvider.ReownWallet)
SDK-->>Playground: Wallet options
Playground->>SDK: ConnectWallet(options)
SDK->>Wallet: Initiate connection (Reown)
Wallet-->>SDK: Session established
SDK-->>Playground: Connected (address, chainId)
Playground->>SDK: Fetch contracts/tokens (updated addresses)
SDK-->>Playground: Data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs. - Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks. - Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Comment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)Assets/Thirdweb/Editor/ThirdwebManagerEditor.cs (2)Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (1)95-126: Avoid blocking the Editor with .Result; use async via delayCall.
Calling Task.Result in OnInspectorGUI can freeze the editor.
- Debug.Log($"Active Wallet ({wallet.GetType().Name}) Address: {wallet.GetAddress().Result}"); + EditorApplication.delayCall += async () => + { + try + { + var address = await wallet.GetAddress(); + Debug.Log($"Active Wallet ({wallet.GetType().Name}) Address: {address}"); + } + catch (System.Exception ex) + { + Debug.LogError($"Failed to get wallet address: {ex}"); + } + };
23-32: Remove stale SupportedChains and IncludedWalletIds references
- In Assets/Thirdweb/Runtime/Unity/ThirdwebManagerBase.cs, remove the IncludedWalletIds/ExcludedWalletIds fields and their [JsonProperty(...)] annotations (around lines 181–185, constructors at 201–204, and usage at 465–468).
- In Assets/Thirdweb/Runtime/Unity/Prefabs/ThirdwebManagerServer.prefab and ThirdwebManager.prefab, clear the <SupportedChains> and <IncludedWalletIds> backing fields.
104-115: Handle connection errors and clarify panel selection mapping.
- Map to a clearer panelProvider name (UI-only), keep provider logic intact.
- Guard against missing panels.
- Wrap ConnectWallet with try/catch and surface errors to the UI.
- private async void ConnectWallet(WalletOptions options) + private async void ConnectWallet(WalletOptions options) { // Connect the wallet - var internalWalletProvider = options.Provider == WalletProvider.MetaMaskWallet ? WalletProvider.ReownWallet : options.Provider; - var currentPanel = WalletPanels.Find(panel => panel.Identifier == internalWalletProvider.ToString()); - Log(currentPanel.LogText, $"Connecting..."); - var wallet = await ThirdwebManager.Instance.ConnectWallet(options); + var panelProvider = options.Provider == WalletProvider.MetaMaskWallet ? WalletProvider.ReownWallet : options.Provider; + var currentPanel = WalletPanels.Find(panel => panel.Identifier == panelProvider.ToString()); + if (currentPanel == null) + { + ThirdwebDebug.LogError($"Missing wallet panel for provider: {panelProvider}"); + return; + } + try + { + Log(currentPanel.LogText, "Connecting..."); + var wallet = await ThirdwebManager.Instance.ConnectWallet(options); // Initialize the wallet panel CloseAllPanels(); // Setup actions ClearLog(currentPanel.LogText); currentPanel.Panel.SetActive(true); @@ currentPanel.Action3Button.onClick.RemoveAllListeners(); currentPanel.Action3Button.onClick.AddListener(async () => { LoadingLog(currentPanel.LogText); var balance = await wallet.GetBalance(chainId: ActiveChainId); var balanceEth = Utils.ToEth(wei: balance.ToString(), decimalsToDisplay: 4, addCommas: true); Log(currentPanel.LogText, $"Balance: {balanceEth} {_chainDetails.NativeCurrency.Symbol}"); }); + } + catch (System.Exception e) + { + Log(currentPanel.LogText, $"Connect failed: {e.Message}"); + } }Also applies to: 119-154
Assets/Thirdweb/Editor/ThirdwebManagerEditor.cs (3)📜 Review detailsAssets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (5)88-90: Improve text area UX (wrap + expand).
Prevent horizontal scrolling and allow vertical expansion.
EditorGUILayout.LabelField("OAuth Redirect Page HTML Override", EditorStyles.boldLabel); -redirectPageHtmlOverrideProp.stringValue = EditorGUILayout.TextArea(redirectPageHtmlOverrideProp.stringValue, GUILayout.MinHeight(150)); +var textAreaStyle = new GUIStyle(EditorStyles.textArea) { wordWrap = true }; +redirectPageHtmlOverrideProp.stringValue = EditorGUILayout.TextArea( + redirectPageHtmlOverrideProp.stringValue, + textAreaStyle, + GUILayout.MinHeight(150), + GUILayout.ExpandHeight(true) +);
130-131: Use HTTPS for docs link.
Minor hardening; avoids mixed-content redirects.
-Application.OpenURL("http://portal.thirdweb.com/unity/v5"); +Application.OpenURL("https://portal.thirdweb.com/unity/v5");
244-246: Tab label: “Server” for server editor.
Current UI says “Client” on the server inspector.
-protected override string[] TabTitles => new string[] { "Client", "Preferences", "Misc", "Debug" }; +protected override string[] TabTitles => new string[] { "Server", "Preferences", "Misc", "Debug" };2-2: Remove unused import.
using System.Threading.Tasks; isn't required in this file. Clean up.
-using System.Threading.Tasks;
96-101: Rename UI field to reflect Reown flow.
The button still reads WalletConnectButton but launches Reown. Rename for clarity and update the prefab wiring.
- [field: SerializeField] - private Button WalletConnectButton; + [field: SerializeField] + private Button ReownWalletButton;- WalletConnectButton.onClick.RemoveAllListeners(); - WalletConnectButton.onClick.AddListener(() => + ReownWalletButton.onClick.RemoveAllListeners(); + ReownWalletButton.onClick.AddListener(() => { var options = GetWalletOptions(WalletProvider.ReownWallet); ConnectWallet(options); });Please rebind the new field in the scene/prefab.
165-168: Provider selection LGTM; consider trace logging.
Logic to pick MetaMask on WebGL with the force flag is fine. Consider a small debug log to ease support triage when switching providers at runtime.
- var externalWalletProvider = Application.platform == RuntimePlatform.WebGLPlayer && WebglForceMetamaskExtension ? WalletProvider.MetaMaskWallet : WalletProvider.ReownWallet; + var externalWalletProvider = + Application.platform == RuntimePlatform.WebGLPlayer && WebglForceMetamaskExtension + ? WalletProvider.MetaMaskWallet + : WalletProvider.ReownWallet; + ThirdwebDebug.Log($"External wallet provider: {externalWalletProvider}");
61-73: Don’t swallow metadata fetch errors.
Capture and log the exception to aid debugging when RPC/metadata fails.
- catch + catch (System.Exception e) { _chainDetails = new ThirdwebChainData() { NativeCurrency = new ThirdwebChainNativeCurrency() { Decimals = 18, Name = "ETH", Symbol = "ETH", }, }; + ThirdwebDebug.LogWarning($"GetChainMetadata failed for {ActiveChainId}: {e.Message}"); }
349-353: Externalize magic contract addresses
Replace the hardcoded ERC-1155 and ERC-20 addresses with serialized string fields—just like ActiveChainId—so users can edit them in the Inspector. ActiveChainId is already serialized and set to Base Sepolia (84532).
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration.
📥 CommitsReviewing files that changed from the base of the PR and between a17e50a and fb3a2bc.
⛔ Files ignored due to path filters (52)Assets/Thirdweb/Runtime/Unity/Wallets/Core/ReownWallet.cs (2)🔇 Additional comments (5)Assets/Thirdweb/Runtime/Unity/Wallets/Core/MetaMaskWallet.cs (2)
- ReownWallet (11-282)
- ReownWallet (21-21)
- MetaMaskWallet (16-352)
- MetaMaskWallet (24-24)
Assets/Thirdweb/Editor/ThirdwebManagerEditor.cs (2)Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (3)2-4: Usings reorder/refs look good.
No semantic impact; aligns with Unity editor conventions.
166-167: Trailing comma OK.
C# allows trailing commas in initializers; keeps diffs cleaner.
69-71: No action needed.
Trailing comma in object initializer is valid in C#; safe to ignore.
28-28: Confirm contracts on Base Sepolia
ActiveChainId correctly set to 84532 and all 421614 references removed. Verify that every hardcoded contract address in Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs and Assets/Thirdweb/Examples/Scenes/Scene_Playground.unity is deployed on Base Sepolia (chain ID 84532).
75-85: Verify Disconnect()’s async signature before refactoring
Cannot locate the Disconnect() definition in the repo – confirm whether it returns a Task. If it’s async, change InitializePanels() to return async void or async Task and await ThirdwebManager.Instance.ActiveWallet.Disconnect();; if it’s synchronous, leave the call as-is.
Sorry, something went wrong.
Introduces ReownWallet integration, including new wallet options and implementation. Updates PlaygroundManager and scene to use new chain IDs and contract addresses, and modifies the editor and prefab to support the new wallet. Also updates assembly references and minor MetaMaskWallet error messages. Closes BLD-205
Set WebglForceMetamaskExtension to 0 in Scene_Playground.unity to prevent forcing the MetaMask extension in WebGL builds.
Simplifies the SetAutoConnectOptions method by removing the WalletProvider.ReownWallet exclusion, allowing auto-connect to proceed regardless of the wallet provider.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (2)📜 Review details81-84: Consider handling disconnect errors gracefully.
While disconnecting the active wallet is good practice, the current implementation doesn't handle potential exceptions from the Disconnect() call. Consider wrapping it in a try-catch to prevent initialization failures.
if (ThirdwebManager.Instance != null && ThirdwebManager.Instance.ActiveWallet != null) { - ThirdwebManager.Instance.ActiveWallet.Disconnect(); + try + { + ThirdwebManager.Instance.ActiveWallet.Disconnect(); + } + catch (System.Exception e) + { + ThirdwebDebug.LogWarning($"Failed to disconnect previous wallet: {e.Message}"); + } }
108-108: Consider adding a comment to explain the wallet provider mapping logic.
The ternary operation that maps MetaMaskWallet to ReownWallet for internal use might be confusing for future maintainers. A comment would help clarify this design decision.
+// Map MetaMaskWallet to ReownWallet internally since MetaMask is handled through Reown in non-WebGL environments var internalWalletProvider = options.Provider == WalletProvider.MetaMaskWallet ? WalletProvider.ReownWallet : options.Provider;
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration.
📥 CommitsReviewing files that changed from the base of the PR and between fb3a2bc and dcd8f14.
⛔ Files ignored due to path filters (48)Assets/Thirdweb/Runtime/Unity/ThirdwebManager.cs (1)🔇 Additional comments (6)Assets/Thirdweb/Runtime/Unity/Wallets/Core/ReownWallet.cs (2)
- ThirdwebManager (7-50)
Assets/Thirdweb/Runtime/Unity/Wallets/Core/MetaMaskWallet.cs (2)
- ReownWallet (11-282)
- ReownWallet (21-21)
- MetaMaskWallet (16-352)
- MetaMaskWallet (24-24)
Assets/Thirdweb/Examples/Scripts/PlaygroundManager.cs (6)2-2: LGTM! Required import for async operations.
The addition of System.Threading.Tasks is necessary for the new async wallet disconnect operation in InitializePanels.
69-70: Minor: Trailing comma added for consistency.
The trailing comma after Symbol = "ETH" follows good practice for multi-line object initializers.
81-84: Good practice: Clean wallet state on initialization.
Disconnecting any active wallet during panel initialization ensures a clean state and prevents potential connection conflicts. This is especially important when switching between wallet providers.
99-99: Successful migration to ReownWallet provider.
The wallet provider has been correctly updated from WalletProvider.WalletConnectWallet to WalletProvider.ReownWallet throughout the code. The implementation properly handles the WebGL MetaMask extension fallback.
Also applies to: 108-108, 165-166
28-28: ActiveChainId updated consistently
ActiveChainId set to 84532 (Base Sepolia), and all references to the old 421614 ID have been removed. Ensure 84532 matches your deployment and that your contracts are deployed on Base Sepolia.
350-350: Confirm updated contract addresses
Ensure the addresses used at lines 350, 373, and 390 (0x8F0a4dde7791fa9B6C62E0B099a1a3ff6dd1cF29 and 0x28C1209fa6e7f1B258Ef65527C94129c6F82995f) are correctly deployed on Base Sepolia (chain ID 84532) and expose the expected ERC-1155 and ERC-20 interfaces respectively.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Introduces ReownWallet integration, including new wallet options and implementation. Updates PlaygroundManager and scene to use new chain IDs and contract addresses, and modifies the editor and prefab to support the new wallet. Also updates assembly references and minor MetaMaskWallet error messages.
Closes BLD-205
Summary by CodeRabbit
New Features
Refactor
Chores