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

First pass at adding profile images. #162

Merged
merged 13 commits into from
Mar 22, 2024
Merged

Conversation

kellan-simiotics
Copy link
Contributor

No description provided.

@kellan-simiotics
Copy link
Contributor Author

@zomglings unsure why the contract fails in the build. I tried a couple different variants of the ITerminus import path. All of them worked locally, but none in the build. Ideas? I don't think we are using web3 anywhere else. Could it be issue with web3 submodule?

Copy link
Contributor

@zomglings zomglings left a comment

Choose a reason for hiding this comment

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

Some questions.

src/Players.sol Outdated Show resolved Hide resolved
src/Players.sol Outdated Show resolved Hide resolved
test/Players.t.sol Outdated Show resolved Hide resolved
src/Players.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@zomglings zomglings left a comment

Choose a reason for hiding this comment

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

lgtm

On second thought, no need to batch the add image and update image methods. If we do need to update how the contract works, we can just deploy new Ballers contracts (e.g. NovaLeagueBallers, BaseLeagueBallers, etc.).

src/Players.sol Outdated
adminTerminus = _adminTerminus;
adminPoolID = _adminPoolID;

ProfileImages[0] = "https://s3.amazonaws.com/static.fullcount.xyz/Beer_League_Ballers/p0.png";
Copy link
Contributor

Choose a reason for hiding this comment

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

CDN URL (https://static.fullcount.xyz/...)

require(terminus.balanceOf(msg.sender, adminPoolID) > 0, "BeerLeagueBallers._enforceIsAdmin: not admin");
}

function addProfileImage(string memory newImage) public {
Copy link
Contributor

Choose a reason for hiding this comment

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

We're probably going to be adding multiple images at once, this seems like it should be a batch method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided not to do this.

NumProfileImages++;
}

function updateProfileImage(uint256 index, string memory newImage) public {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also probably be a batch method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We decided not to do this.

test/Players.t.sol Show resolved Hide resolved
@kellan-simiotics kellan-simiotics merged commit b94bd7a into main Mar 22, 2024
1 check passed
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