getFullCanvas #39
getFullCanvas #39
Conversation
|
@melanke is attempting to deploy a commit to the BuidlGuidl Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Thanks for the updates!
Just a quick note on the contract (though I’m not a full-on smart contract expert): the way you’re retrieving the full canvas doesn’t seem optimal. It’s quite gas inefficient, and you’ve reintroduced the "big array" again. We moved to mappings because they’re more efficient, but your getFullCanvas function switches back to arrays.
As a general rule of thumb, try to keep the smart contract as lean as possible and shift the heavy lifting to the front-end.
For example, could you use emitted events in the front-end to display the canvas instead?
| * @dev Initialize a default Buidlguidl Batch11 inspired pixel art | ||
| * This is a simplified representation and can be customized | ||
| */ | ||
| function initializeBuidlGuidlLogo() private { |
There was a problem hiding this comment.
It's very repetitive, why not having a loop? What costs gas is writing to the blockchain.
| function getFullCanvas() external view returns (Pixel[][] memory) { | ||
| Pixel[][] memory fullCanvas = new Pixel[][](CANVAS_WIDTH); | ||
|
|
||
| for (uint256 x = 0; x < CANVAS_WIDTH; x++) { | ||
| fullCanvas[x] = new Pixel[](CANVAS_HEIGHT); | ||
| for (uint256 y = 0; y < CANVAS_HEIGHT; y++) { | ||
| fullCanvas[x][y] = canvas[x][y]; | ||
| } | ||
| } | ||
|
|
||
| return fullCanvas; | ||
| } |
There was a problem hiding this comment.
In here you reintroducing the array again, we wanted to get rid of and exchanged it with a mapping.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Merged! I just deployed the contract and updated the ABI accordingly in |
Description
Additional Information
Your ENS/address:
melanke.eth