-
Notifications
You must be signed in to change notification settings - Fork 8
Initial Commit #296
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
base: master
Are you sure you want to change the base?
Initial Commit #296
Conversation
Wallet, deploy and mint test. Editor helper not working
update to enable editor calls reading json body correctly
@@ -0,0 +1,7 @@ | |||
using UnityEngine; | |||
|
|||
[CreateAssetMenu(fileName = "SidekickConfig", menuName = "ScriptableObjects/SidekickConfig")] |
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.
MenuName should be "Sequence/SidekickConfig" - this way it lives with our other scriptable objects (including the others in this PR)
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 should probably live somewhere else 😉
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.
If we want to ship this to users, it should go somewhere in the Packages folder
} | ||
} | ||
|
||
[MenuItem("Sequence Dev/Start Sidekick", false, 0)] |
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.
I wonder if maybe "Sequence/" is a better menu item path for these - this way we can leave "Sequence Dev" for us. What do you think?
[CreateAssetMenu(fileName = "SidekickConfig", menuName = "ScriptableObjects/SidekickConfig")] | ||
public class SidekickConfig : ScriptableObject | ||
{ | ||
public string path; |
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.
What should the path be? I tried http://localhost:8080/
but received the following error:
"Sidekick path not set or invalid: http://localhost:8080/"
DeployResult deployResult = JsonUtility.FromJson<DeployResult>(result); | ||
|
||
if (!string.IsNullOrEmpty(deployResult.result.txHash)) | ||
{ | ||
|
||
var receipt = await new SequenceEthClient(sidekick.Chain).WaitForTransactionReceipt(deployResult.result.txHash); | ||
|
||
if (receipt != null) | ||
{ | ||
string deployedAddress = ReceiptExtractor.ExtractFirstContractAddressExceptOwn(receipt, DeployERC20InitialOwner); | ||
if (!string.IsNullOrEmpty(deployedAddress)) | ||
{ | ||
Debug.Log($"Deployed contract address: {deployedAddress}"); | ||
} | ||
else | ||
{ | ||
Debug.LogWarning("Could not extract deployed contract address from receipt."); | ||
} | ||
} | ||
else | ||
{ | ||
Debug.LogError("Failed to get transaction receipt."); | ||
} | ||
} | ||
else | ||
{ | ||
Debug.LogError($"Deployment error: {deployResult.result.error}"); | ||
} |
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 logic is repeated a few times - consider using a helper function
public async void SafeMintERC721() | ||
{ | ||
var payload = new SafeMintERC721Payload | ||
{ | ||
to = SafeMintERC721Recipient, | ||
tokenId = SafeMintERC721TokenId | ||
}; | ||
string jsonString = JsonUtility.ToJson(payload); | ||
|
||
Debug.Log("Calling safeMint ERC721: " + jsonString); | ||
|
||
try | ||
{ | ||
string result = await sidekick.SafeMintERC721(SafeMintERC721ContractAddress, jsonString); | ||
Debug.Log("safeMint result: " + result); | ||
} | ||
catch (System.Exception ex) | ||
{ | ||
Debug.LogError("safeMint ERC721 failed: " + ex.Message); | ||
} | ||
} | ||
|
||
public string[] SafeMintERC721BatchRecipients { get; set; } | ||
public string[] SafeMintERC721BatchTokenIds { get; set; } | ||
|
||
public async void SafeMintBatchERC721() | ||
{ | ||
var payload = new SafeMintBatchERC721Payload | ||
{ | ||
recipients = SafeMintERC721BatchRecipients, | ||
tokenIds = SafeMintERC721BatchTokenIds | ||
}; | ||
string jsonString = JsonUtility.ToJson(payload); | ||
|
||
Debug.Log("Calling safeMintBatch ERC721: " + jsonString); | ||
|
||
try | ||
{ | ||
string result = await sidekick.SafeMintBatchERC721(SafeMintERC721ContractAddress, jsonString); | ||
Debug.Log("safeMintBatch result: " + result); | ||
} | ||
catch (System.Exception ex) | ||
{ | ||
Debug.LogError("safeMintBatch ERC721 failed: " + ex.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.
Unlike the other mint functions, you didn't wrap these in an EditorApplication.delayCall
. Should you have?
public string ToHexString(string input) | ||
{ | ||
byte[] bytes = Encoding.UTF8.GetBytes(input); | ||
StringBuilder hex = new StringBuilder("0x"); | ||
|
||
foreach (byte b in bytes) | ||
{ | ||
hex.Append(b.ToString("x2")); | ||
} | ||
|
||
return hex.ToString(); | ||
} |
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.
Let's put this in StringExtensions for further re-usability
{ | ||
private static readonly HttpClient client = new(); | ||
private string baseUrl = "http://localhost:3000"; | ||
private string secretKey = "ultrasecretkey"; |
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.
What is this used for?
catch (HttpRequestException e) | ||
{ | ||
throw; | ||
} |
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.
Redundant
Great demo! Some great ideas in here too. Lots of really good stuff for us to build off of in the future. Left some comments and questions for you to help with readability and integration. |
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.
These tasks/API calls would be much easier to integrate (and less error prone) if the methods took in objects as parameters instead of json (same goes for returning)
Fixes to scripts locations
Wallet, deploy and mint test. Editor helper not working
Version Increment
Please ensure you have incremented the package version in the package.json as necessary.
Docs Checklist
Please ensure you have addressed documentation updates if needed as part of this PR: