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

General fixes #1399

Open
5 of 28 tasks
kevinunger opened this issue May 19, 2022 · 7 comments
Open
5 of 28 tasks

General fixes #1399

kevinunger opened this issue May 19, 2022 · 7 comments
Assignees

Comments

@kevinunger
Copy link
Contributor

kevinunger commented May 19, 2022

1.) Frontend / Ui / Ux

1.1 ) General

1.2 ) Blacksmith / Weapon List

  • 1.2.1) A user reported that the left nav-bar is missing on mobile. Couldn't reproduce so far. Seems to be a race condition type thing or an error that happens when navigating the site.

image

  1. Go to blacksmith
  2. Forge a weapon
  3. Accept the first skill approval
  4. Decline the second mint approval
  5. Dust your weapons

1.3) Raid

  • 1.3.1) When claiming rewards, weapons aren't shown most of the time, but junk/trinkets/dust are.

1.4) Combat

1.5) Character bar

1.6) Staking

2.) Errors

2.1) General

2.2) Combat

  • 2.2.1) Revert Error after fight

image

Error: Transaction has been reverted by the EVM:
{
  "blockHash": "0xa0283e159ebcfb59556e916934ca6aaac546d8514cd56fc1befe97a75d5284ce",
  "blockNumber": 21692922,
  "contractAddress": null,
  "cumulativeGasUsed": 872582,
  "from": "0xab47675fdb7ae6e47431bd2d4bd9cd7bac7fde61",
  "gasUsed": 43324,
  "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
  "status": false,
  "to": "0x4ed213d2f40380bbb0c8b4231acfa7defa5ccc25",
  "transactionHash": "0xf6fecadbace894cd9d5edf179f3591e73cb989662d3d8e590692038eba9cfa1d",
  "transactionIndex": 5,
  "type": "0x0",
  "events": {}
}
    at Object.TransactionError (errors.js:87:21)
    at Object.TransactionRevertedWithoutReasonError (errors.js:98:21)
    at index.js:394:57
    
2.3) Plaza ---

image

    async updateMintCharacterFee() {
      ...
      const skillRecruitCost = await this.contracts.CryptoBlades.methods.usdToSkill(recruitCost).call();
      ...
    }

(It's called on created(). So contracts is probably still undefined. Needs a check for contracts to exist)

FIX:
Implement a watcher for defaultAccount like I did here:
https://github.com/CryptoBlades/cryptoblades/pull/1549/files

This will prevent the error and errors resulting from that. Guards can be removed too.

2.4) Leaderboard

  • 2.4.1) Not loading?

2.5) Blacksmith

  • 2.5.4) No confirmation on salvage
  • 2.5.6) Weapon list is not updated after salvage
  • 2.5.7) Salvaging cost is sometimes 0

2.6) Dashboard

  • 2.6.1) "Registration Ends" Time Remaining only shows Hour and Minutes (it should also display and Days, and Second).

image

2.7) Treasury

  • 2.7.1) Display Errors on Treasury

image

The amount of claimed tokens seems to be missing in the first line and the amount is not displayed in the second line.

2.8) Bridge

  • 2.8.1) Experience numbers displayed of characters are not correct (after bridging?) in the storage tab of /bridge

Notice that the bars are probably correct, but not the exp numbers.

image

Suggestions, not bugs:

  • 1.) - Bazaar icon text has an icon besides it when in the navbar, but not in the menu on the right ( 'Wiki' should have the same icon, bc it's an external link too)

  • 10.) Add font color here depending on the rarity and icon to Element: ... #1509

  • 3.) - The weapons list shows all excluding the ones marked as favorites, when the 'show favorites' filter is disabled and shows all weapons when 'show favorites' filter is enabled
    Wouldn't it be more logical if we would show only the favorites when clicking the 'show favorites button' ?

  • 5.) - A Tooltip explaining the raid shortly would be great.

  • 7.) There is a hardcoded delay for this message:

image

It's a bit confusing when this flies in seemingly randomly. I'd like to remove the delay or the message entirely.

  • 8.) There should be a 'deselect all' btn when selecting multiple nfts e.g. when salvaging. E.g. if you select multiple swords, change filters, etc. you need to remove all filters and deselect all select by hand again.
  • 10.) Add font color here depending on the rarity and icon to Element: ...

image

  • 11.) Regarding 1.2.12) : Do we really want to display the weapons like that on hover? What kind of benefit does that provide having them 5% bigger and with a different layout (Property Element moves to the bottom, Stars are next to battle power) and it's obstructing the other weapons. Maybe an 'inspect'-icon would be better. That enlarges the weaponns (nfts in general) and shows more stats like on bazaar

  • 12.) Capitalize 'Nft Display ' to 'NFT Display' #1489

image

  • 13.) our asset folder is at ~100MB. Think it would be reasonable to clear that a bit and check for unused files and to convert big .pngs we load on the site to smaller .webp (e.g. the big background, character art etc.)

  • 14.) there might be some kind of issue with the type checking. The type checking service is limited to 2Gb memory, which get's full pretty quickly. there might be some kind of leak or just too much to check. We could just increase the memory or better, check why it's happening and fix that. This might be resolved by going from webpack to vite

  • 15.) We could lazy load parts that aren't active on startup. This can be done in router.ts to lazy load views, via dynamic imports for components and in store.ts to lazy load vuex modules when they are needed or use webpack chunks

@Sellie96
Copy link
Contributor

My word Kevin, extensive list. Few of them I have notes on

1.1.1) Why are only 4 nav-links on the nav-bar? We are redoing the claim section into that bar, this will lead to it being more full, until that time we are limiting the number shown we can increase it when we know the available space. This covers a few more points too.

1.2.6) They are just always shown now. We can have it back but needs defauting to true, it was annoying explaining to people they aren't missing weapons.

1.3.2) Did raids fall over? This was not the case before

1.3.6) See point 1. Still needs rework

6.) - Should we remove 'Merchandise' completly? Absolutely. Legacy code needs cleaning

May have missed some explainable ones, but generally I agree with most of the others. Good work identifying these

@poshdan
Copy link
Collaborator

poshdan commented May 20, 2022

wanted to add, victory chance on raids has been irrelevant since september. It doesn't need to be shown unless we reintroduce it (not until raids v2, if ever)

@poshdan
Copy link
Collaborator

poshdan commented May 22, 2022

On the staking page, the default behavior is for the numbers to load as 0.
Could we add a loading circle somewhere (and hide the 0s) so people don't freak out that their tokens/rewards are gone before the rpc responds?

EDIT while on the topic, I figured I'd mention that having some token icons and an arrow between them (what you get for what stake) would go a long way to understand the options at a glance without reading the text.
And I wanted to make sure that there is some clear visual cue for when the rewards are depleted or if the distribution time is over (without having to have tokens locked, that's when the bar appears only)

@kevinunger
Copy link
Contributor Author

Ok, added your first point to the issue list!

for the second one:
You mean adding the token icons on the top and having an arrow here?
image

Last one. Added to the list.

@seiyria
Copy link
Contributor

seiyria commented Jun 10, 2022

please allow me to get help addressing: 2.5.5, suggestions.2, suggestions.4, as well as a new suggestion: capitalizing Nft Display to NFT Display

(if this comment is older than 2 weeks, disregard this)

@kevinunger
Copy link
Contributor Author

@seiyria

@remocwenn
Copy link

Anything that is not yet resolved, but is still needing to be resolved needs to be put into their own issues.

@kevinunger kevinunger changed the title Ongoing issues Pay your devs Jun 9, 2023
@MrBaptista69 MrBaptista69 changed the title Pay your devs General fixes Jun 19, 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

No branches or pull requests

7 participants