momentum-mod / website

Momentum Mod's main website.
https://momentum-mod.org
MIT License
58 stars 61 forks source link

fix(utils): fix negative current exp bug #1060

Closed petrecheli closed 3 weeks ago

petrecheli commented 3 weeks ago

Closes #1045

Fix negative current exp

Screenshot_2

Checks

tsa96 commented 3 weeks ago

Doesn't seem right, this produces the wrong values for other levels - see unit tests in xp-systems.class.spec.ts. You can run those tests with nx test xp-systems.

What I would do is add the line { level: 1, xpInLevel: 21000, total: 21000 }, on line of that file, so we get a test case that fails (appropriately) with the current system.

 for (const { level, xpInLevel, total } of [
      { level: 1, xpInLevel: 21000, total: 21000 },
      { level: 2, xpInLevel: 22000, total: 43000 },
      { level: 3, xpInLevel: 23000, total: 66000 },
      { level: 4, xpInLevel: 24000, total: 90000 },
...
    ]) {
      expect(service.getCosmeticXpInLevel(level)).toBe(xpInLevel);
      expect(service.getCosmeticXpForLevel(level)).toBe(total);
    }
petrecheli commented 3 weeks ago

Look at my new commit, I kept the function the same and made changes directly to the XP calculations that I found for the front end, now the test continues to work and I also tested manually on the frontend and it is correct. See if there is anything wrong.

tsa96 commented 3 weeks ago

Following discussion yesterday, I'm going to close this in favor of my PR, which I believe addresses the root cause of the problem.

Sorry to discard of this - though even if it didn't result in a merged PR, your changes and input on Discord nonetheless helped clarify a lot for me, so it was a really helpful contribution. Appreciate the time you put into this and happy to suggest/find more stuff to work on if you're interested!