-
Notifications
You must be signed in to change notification settings - Fork 78
Connect wallet button prefab #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adding Enums.cs at the SDK level might help bridge the gap between high level calls and inner classes |
private void Awake() | ||
{ | ||
#if !UNITY_EDITOR | ||
SDK = new ThirdwebSDK(chain.ToString().ToLower()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this wont work with chains names that have a -
like optimism-goerli
. One thing that might help for this is that we're about to merge a PR in the TS SDK that now accepts ChainID as the network input. So that will work nicely with an enum. For now I think passing string is safer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point me to where I can find the expected strings? But yes chain ID as the network input would solve it.
|
||
void LogThirdweb(string _message) | ||
{ | ||
Debug.Log($"[Thirdweb] {_message}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one thing that ive been thinking of is a global flag to enable / disable logs in the SDK. Wonder if these might get a bit verbose? fine for now tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having logging as an option is probably a good idea, would remove a bunch of logs the dev would typically add anyway.
Vector3 targetScale = dropdownExpand ? Vector3.one : Vector3.zero; | ||
while (Math.Abs(_object.localScale.x - targetScale.x) > 0.01f) | ||
{ | ||
_object.localScale = Vector3.Lerp(_object.localScale, targetScale, 10f * Time.deltaTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prob don't need to animate here. if we do then let's make it a simple fade
}); | ||
|
||
connectInfoText.text = $"{_wallet} ({chain})"; | ||
walletAddressText.text = $"Connected As: {address}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we shorten the address? it makes the button really large otherwise. We can have a util that shortens to 0x123...456
Added |
public List<Wallet> supportedWallets; | ||
public string chain = "ethereum"; | ||
public int chainID = 1; | ||
public List<Wallet> supportedWallets = new List<Wallet> { Wallet.MetaMask, Wallet.CoinbaseWallet, Wallet.Injected, Wallet.MagicAuth }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove Injected / Magic, add WalletConnect - default should be MM / CBW / WC
} | ||
|
||
// Utility | ||
|
||
int GetChainID(Chain _chain) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonder if it's worth leaving this but using string (could also help validate the inputted string)
just until we ship the new chain id SDK input
Vanilla version of Prefab_ConnectWallet