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

Added Purse API and Purse Change Event #950

Merged
merged 7 commits into from
Nov 23, 2024

Conversation

Westsi
Copy link
Contributor

@Westsi Westsi commented Aug 21, 2024

I added a way to create callbacks for when the player's purse changes and fixed the Utils.getPurse function so that it would not break in the case below. This so that the Purse API can be used to track scavenger coins, useful in profit calculators for slayers and ghosts for instance.

The case that broke the getPurse function:
Purse: 372,647 works fine, as the prior regex changed that to 372647.
However, when the purse is being updated with mob kill coins, the line becomes Purse: 372,768 (+121), where the prior regex just removed all non digits, making it 372768121.
To fix this, I added a regex before the existing regex that matches the (+nnn) section and removes it.

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Aug 21, 2024
@viciscat
Copy link
Collaborator

This seems a bit of a little random addition (apart from the regex fix)?

@Westsi
Copy link
Contributor Author

Westsi commented Aug 21, 2024

Yeah, I was working on a slayer profit calculator and saw that there was no way of tracking purse changes like this. I thought that it was best to make it a more abstract thing so that it could be used in other similar calculators (ghost profit, bountiful reforge in farming etc). I maybe should have explained that more.

@AzureAaron AzureAaron added this to the 1.23.0 milestone Aug 30, 2024
@AzureAaron AzureAaron added the merge conflicts This PR has merge conflicts that need solving. label Sep 4, 2024
@Westsi
Copy link
Contributor Author

Westsi commented Sep 6, 2024

Merge conflict looks fairly simple to fix to me.

@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews merge conflicts This PR has merge conflicts that need solving. changes requested This PR need changes and removed reviews needed This PR needs reviews merge conflicts This PR has merge conflicts that need solving. labels Sep 8, 2024
@Westsi
Copy link
Contributor Author

Westsi commented Sep 14, 2024

Done @kevinthegreat1

@kevinthegreat1
Copy link
Collaborator

This pr needed to be rebased. I have done that for you. I apologize for the very rapidly changing main branch, but that's bound to happen to an interdependent project like this, so it would be nice if you could deal with these situations.

@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Sep 22, 2024
@kevinthegreat1
Copy link
Collaborator

This api was a bit messy. I have rewritten it. Feel free to take a look but it should be good now.

kevinthegreat1
kevinthegreat1 previously approved these changes Sep 22, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Sep 22, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels Sep 22, 2024
kevinthegreat1
kevinthegreat1 previously approved these changes Sep 22, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Sep 22, 2024
@AzureAaron AzureAaron added the merge conflicts This PR has merge conflicts that need solving. label Oct 21, 2024
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Nov 16, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Nov 22, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Nov 22, 2024
@kevinthegreat1
Copy link
Collaborator

Note

Squash

@AzureAaron AzureAaron merged commit beeb590 into SkyblockerMod:master Nov 23, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Nov 23, 2024
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.

5 participants