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 Speed Status Bar #855 #945

Merged
merged 5 commits into from
Sep 6, 2024

Conversation

Westsi
Copy link
Contributor

@Westsi Westsi commented Aug 19, 2024

In order to solve the issue #855, I implemented a speed status bar in the way that was discussed in that issue (r.e. speed calculations), and added it to all the same places as the existing health, mana, defence, and experience bars. I created a texture for it.

I have tested the bar and most of the exceptions to the 400 speed cap in game. I have not tested the racing helmet nor the black cat pet as I don't have the money for that, but since they work in the same way as the other exceptions I see no reason why they shouldn't work.

I tested:

  • Cactus Knife (only in garden)
  • Snail pet
  • Young Dragon Armour

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Aug 19, 2024
@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label Aug 20, 2024
@AzureAaron AzureAaron added this to the 1.23.0 milestone Aug 20, 2024
viciscat
viciscat previously approved these changes Aug 21, 2024
Copy link
Collaborator

@viciscat viciscat left a comment

Choose a reason for hiding this comment

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

It works well and the code looks pretty good so uh good job 👍
Did not test the max speed exceptions tho.

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Aug 21, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

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

Code looks great; just one thing.

@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed merge me please Pull requests that are ready to merge labels Aug 21, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Aug 21, 2024
kevinthegreat1
kevinthegreat1 previously approved these changes Aug 21, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

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

Works well. Except this is what new players see lol.
Screenshot 2024-08-22 at 02 10 17

@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed reviews needed This PR needs reviews labels Aug 21, 2024
@Westsi
Copy link
Contributor Author

Westsi commented Aug 21, 2024

I looked at that but I don't think that it's any bigger of an issue than it was before this change. I could change the default positioning of it, but at that point why not change the defaults of all of them?

@kevinthegreat1
Copy link
Collaborator

Yeah. I'm just leaving that here in case someone wants to modify the defaults for all the bars. We can merge this pr already.

@kevinthegreat1 kevinthegreat1 added the good first issue Welcome new contributors :) label Aug 21, 2024
@AzureAaron AzureAaron linked an issue Aug 22, 2024 that may be closed by this pull request
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge me please Pull requests that are ready to merge labels Sep 6, 2024
Copy link
Collaborator

@kevinthegreat1 kevinthegreat1 left a comment

Choose a reason for hiding this comment

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

Changed the defaults a bit.

@LifeIsAParadox LifeIsAParadox removed the reviews needed This PR needs reviews label Sep 6, 2024
@LifeIsAParadox LifeIsAParadox added the merge me please Pull requests that are ready to merge label Sep 6, 2024
@kevinthegreat1 kevinthegreat1 merged commit 04afe1a into SkyblockerMod:master Sep 6, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Sep 6, 2024
@kevinthegreat1
Copy link
Collaborator

Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Welcome new contributors :) new feature This issue or PR is a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✦ Speed Status Bar
5 participants