Skip to content
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

Add media metadata renderer #108

Closed
wants to merge 7 commits into from

Conversation

neokry
Copy link
Collaborator

@neokry neokry commented Jul 24, 2023

Adds new metadata renderer and allows for custom metadata renderer

  • adds a function to manager to deploy and replace a DAOs metadata renderer
  • adds the media metadata renderer
  • adds a new function to property renderer to generate artwork for specific tokens (useful for daos that are coming from a different renderer)

Code review

  • should updating the renderer be done in a different way?

@neokry neokry changed the base branch from main to v2-generic-manager July 24, 2023 06:34
@neokry neokry temporarily deployed to Test July 31, 2023 07:09 — with GitHub Actions Inactive
@neokry neokry temporarily deployed to Test July 31, 2023 08:43 — with GitHub Actions Inactive
@neokry neokry changed the title V2 media metadata renderer Media metadata renderer Aug 2, 2023
@neokry neokry changed the title Media metadata renderer Add media metadata renderer Aug 2, 2023
@neokry neokry temporarily deployed to Test August 2, 2023 04:48 — with GitHub Actions Inactive
@neokry neokry temporarily deployed to Test August 7, 2023 06:35 — with GitHub Actions Inactive
@neokry neokry temporarily deployed to Test August 7, 2023 06:38 — with GitHub Actions Inactive
@neokry neokry requested a review from iainnash August 7, 2023 06:49
@neokry neokry temporarily deployed to Test August 17, 2023 03:59 — with GitHub Actions Inactive
address _newRendererImpl,
bytes memory _setupRenderer
) external returns (address metadata) {
if (msg.sender != IOwnable(_token).owner()) revert ONLY_TOKEN_OWNER();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's nice to always use { and } brackets to prevent bugs and readability.

Copy link
Contributor

@iainnash iainnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall structure looks good – left some comments.

tokenAttributes[i + 1] = uint16(seed % numItems);

// Adjust the randomness
seed >>= 16;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this comment is a bit strange – get next 16 random bits likely makes more sense.

@@ -50,6 +50,19 @@ interface IPropertyIPFSMetadataRenderer is IBaseMetadata, MetadataRendererTypesV
///
error TOO_MANY_PROPERTIES();

error TOKEN_ALREADY_GENERATED(uint256 tokenId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this error?

@neokry neokry closed this Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants