Skip to content

Bug fixes#113

Open
Megha-Dev-19 wants to merge 7 commits into
devfrom
bug-fixes
Open

Bug fixes#113
Megha-Dev-19 wants to merge 7 commits into
devfrom
bug-fixes

Conversation

@Megha-Dev-19

@Megha-Dev-19 Megha-Dev-19 commented Nov 24, 2024

Copy link
Copy Markdown
Collaborator
  • Scoreboard clicking plus it resets back to 5.
  • Cheddy does not show wen board loads (rare)
  • Fix the navbar highlight line on collapse
  • Allow mint on every 100 cheddar, instead of minting on every win, backend
  • Disable user to play after they have reached daily limit

Not able to recreated / removed from this PR

  • Leaderboard score wrong showing 555 is showing 545 seems to be on a refresh interval.
  • Game jumps in CoinBase Wallet
  • Board paints a cell that can not be reached (rare)
  • Board is not changing layout on new games

@Megha-Dev-19 Megha-Dev-19 marked this pull request as draft November 24, 2024 17:27
@netlify

netlify Bot commented Nov 24, 2024

Copy link
Copy Markdown

Deploy Preview for test-cheddar-ecosystem ready!

Name Link
🔨 Latest commit e864e93
🔍 Latest deploy log https://app.netlify.com/sites/test-cheddar-ecosystem/deploys/675fdbf9d7e36d00097e85cd
😎 Deploy Preview https://deploy-preview-113--test-cheddar-ecosystem.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Megha-Dev-19 Megha-Dev-19 marked this pull request as ready for review November 27, 2024 15:43

@Mart-Dan-Rossi Mart-Dan-Rossi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In addition to comments made, i found that, even when some texts and styles were changed pointing that after a match the cheddar earned should be not minted inmidiatly and claimed all toghether after clicking a button, this is not happening.

Comment thread src/app/globals.css Outdated
Comment thread src/components/maze/GameOverModalContent.tsx Outdated
Comment thread src/components/maze/GameOverModalContent.tsx Outdated
Comment thread src/components/maze/GameboardContainer.tsx
Comment thread src/contexts/maze/GameContextProvider.tsx
Comment thread src/components/maze/Gameboard.tsx Outdated

@RodrigoCSolari RodrigoCSolari left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with the changes that Martin asked for.

@Megha-Dev-19

Copy link
Copy Markdown
Collaborator Author

@Mart-Dan-Rossi @RodrigoCSolari I have resolved the comments, and removed the board related changes (randomness and unreachable cell), so we can merge the other fixed bugs.

Comment thread src/components/maze/Gameboard.tsx Outdated
: 'playerBackgroundElementOnTop';

// Combine multiple CSS classes using the 'clsx' utility function to handle conditional styling
return clsx(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We have multiple lines in the project where we should use this function to be consistent on the way to do this stuff. I know it's not a priority right now to refactor the code but it doesn't look good idea to comment every place where we use this function. I was expecting some better documentation, like creating a local function just to call the clsx but adding a description so it could be seen while hovering the function on the code or something like that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we can do that in refactoring session.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah but if that can be done in refactoring season, ¿Why did you change this now? ¿Why did you add clsx function in this PR?

It's not a braking thing, the code will be as good as before but let's try to not do PR's with so many tasks because this particular change is not compleate/shouldn't be done now i think.

<div className={styles.mazeContainer} ref={gameboardRef} tabIndex={0}>
<div className={styles.toolbar}>
{earnedButNotMintedCheddar > 0 && (
{earnedButNotMintedCheddar > 100 && (

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I forgot to check this. Wait till i do plz.

@Mart-Dan-Rossi Mart-Dan-Rossi Dec 10, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm having issues starting a match using local env and bundle-mint branch running. Any idea what could cause this?
Uploading Captura de pantalla 2024-12-10 164732.png…

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Captura de pantalla 2024-12-10 164745
Uploading Captura de pantalla 2024-12-10 164732.png…

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Captura de pantalla 2024-12-10 164732

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@Mart-Dan-Rossi did you remove the existing data from /db, because the structure of data changed after we added base, so maybe that might be the reason?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

https://drive.google.com/file/d/1Ps9CdDtGCxzKrZTVn42GeyM0a-OF9Rqo/view?usp=sharing

Yeah, that's what i saw. I was thinking that if we can't make the button work properly BECAUSE we don't have the mint keys this functionallity is flat wong. In production users won't have mint keys.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oh that's because the backend server has the keys, so the frontend user don't need it. It will work once we deploy the code to production.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmmm.. And if it doesn't? I'm not sure how to handdle this situation. @RodrigoCSolari any suggestion?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I guess, he can deploy it on testnet (the backend, which has the keys) and then we can test it.

@Mart-Dan-Rossi

Copy link
Copy Markdown
Collaborator

image

Critical bug!

In some matches you just can't find the door.

image

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.

3 participants