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.

Sign up for GitHub

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

Jump to bottom

Slayer System rework #1040

Merged
merged 53 commits into from
Dec 23, 2024
Merged

Slayer System rework #1040

merged 53 commits into from
Dec 23, 2024

Conversation

7azeemm
Copy link
Contributor

@7azeemm 7azeemm commented Nov 1, 2024

  • Added Slayer HUD
  • Added Boss slain time
  • Added Personal Best slain time
  • Added Boss and MiniBoss spawn alert
  • Added Mute Enderman sounds
  • Added Lazer phase Timer

@7azeemm
Added Slayer HUD
8d04e75 
Added Boss slain time
Added Personal Best slain time
Added Boss and MiniBoss spawn alert
Added Mute Enderman sounds
Added Lazer phase Timer
@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Nov 1, 2024
@7azeemm
Copy link
Contributor Author

7azeemm commented Nov 1, 2024

image
just need help fixing this
i didn't figure out how to make text renders behind entity
also glow effect is broken (filled not outline like in img) in beta 1 for only 1.21.3 minecraft version, beta 1 1.21.1 works fine. (same problem without sodium)

idk if that's a good color combo in hud
i'm bad at designing
image

@Emirlol
Copy link
Collaborator

Emirlol commented Nov 1, 2024

Looks pretty good.
I went ahead and fixed the filled outline issue, along with a few other things that kept crashing my game so that I could actually test it. Hope you don't mind.

@7azeemm
Copy link
Contributor Author

7azeemm commented Nov 1, 2024

nice, appreciate your help

Emirlol
Emirlol previously requested changes Nov 1, 2024
View reviewed changes
Copy link
Collaborator

@Emirlol Emirlol left a comment

Choose a reason for hiding this comment

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

A few nitpicks here and there but the code looks good in general. I didn't look too deep into the logic since stuff just seemed to work, excluding the LazerTimer as I wasn't able to test it. (too weak for voidgloom seraph lol)

src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/hud/SlayerHudWidget.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/skyblock/slayers/SlayerManager.java Outdated Show resolved Hide resolved
src/main/resources/assets/skyblocker/lang/en_us.json Outdated Show resolved Hide resolved
src/main/resources/assets/skyblocker/lang/en_us.json Outdated Show resolved Hide resolved
src/main/resources/skyblocker.mixins.json Outdated Show resolved Hide resolved
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed reviews needed This PR needs reviews labels Nov 1, 2024
@Emirlol Emirlol added the new feature This issue or PR is a new feature label Nov 1, 2024
kevinthegreat1
kevinthegreat1 reviewed Nov 4, 2024
View reviewed changes
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.

Not a complete review yet, but just some comments.

src/main/java/de/hysky/skyblocker/mixins/SoundSystemMixin.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/mixins/WorldRendererMixin.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/mixins/WorldRendererMixin.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/mixins/WorldRendererMixin.java Outdated Show resolved Hide resolved
src/main/java/de/hysky/skyblocker/utils/Utils.java Outdated Show resolved Hide resolved
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed changes requested This PR need changes labels Nov 4, 2024
@7azeemm
Copy link
Contributor Author

7azeemm commented Nov 9, 2024

Looks good to me

@AzureAaron AzureAaron linked an issue Dec 20, 2024 that may be closed by this pull request
Vampire slayer Steak stake and twinclaws alert not work #1033
Closed
@BigloBot
Copy link
Contributor

BigloBot commented Dec 21, 2024 •
edited
Loading

There's a regression where the slayer bossbar won't redetect the new slayer armourstand if the player moves out and back in of range of the slayer. I think you an just dynamically attempt to grab it here if the boss fight is active

AzureAaron
AzureAaron requested changes Dec 21, 2024
View reviewed changes
Copy link
Collaborator

@AzureAaron AzureAaron left a comment

Choose a reason for hiding this comment

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

just one small thing

src/main/java/de/hysky/skyblocker/skyblock/slayers/boss/voidgloom/LazerTimer.java Outdated Show resolved Hide resolved
@kevinthegreat1
Copy link
Collaborator

There's a regression where the slayer bossbar won't redetect the new slayer armourstand if the player moves out and back in of range of the slayer. I think you an just dynamically attempt to grab it here if the boss fight is active

I think we can just make checkSlayerBoss process new armor stands when the current slayer boss armor stand is removed or is not alive or however we check for the armor stand being out of range.

@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 22, 2024 •
edited
Loading

There's a regression where the slayer bossbar won't redetect the new slayer armourstand if the player moves out and back in of range of the slayer. I think you an just dynamically attempt to grab it here if the boss fight is active

Should be fixed now, I added Scoreboard check for boss spawning if player was not in range of boss, but only starts the timer if enabled and spawn alert. the bossbar and glow won't work because onEntityTrackerUpdate works only in specific range not in all the world

@7azeemm
Copy link
Contributor Author

7azeemm commented Dec 22, 2024

alr

kevinthegreat1
kevinthegreat1 previously approved these changes Dec 22, 2024
View reviewed changes
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed changes requested This PR need changes labels Dec 22, 2024
AzureAaron
AzureAaron previously approved these changes Dec 22, 2024
View reviewed changes
@kevinthegreat1 kevinthegreat1 dismissed stale reviews from AzureAaron and themself via 64ee6e1 December 23, 2024 02:42
@LifeIsAParadox LifeIsAParadox added changes requested This PR need changes and removed merge me please Pull requests that are ready to merge labels Dec 23, 2024
@LifeIsAParadox LifeIsAParadox added merge me please Pull requests that are ready to merge and removed changes requested This PR need changes labels Dec 23, 2024
@kevinthegreat1 kevinthegreat1 merged commit c4f9bc4 into SkyblockerMod:master Dec 23, 2024
1 check passed
@LifeIsAParadox LifeIsAParadox removed the merge me please Pull requests that are ready to merge label Dec 23, 2024
@7azeemm 7azeemm deleted the SlayerSystem branch December 23, 2024 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BigloBot BigloBot BigloBot left review comments

@kevinthegreat1 kevinthegreat1 kevinthegreat1 approved these changes

@Emirlol Emirlol Emirlol left review comments

@AzureAaron AzureAaron AzureAaron left review comments

Assignees
No one assigned
Labels
new feature This issue or PR is a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vampire slayer Steak stake and twinclaws alert not work
7 participants
@7azeemm @Emirlol @kevinthegreat1 @x-yingcan-x @BigloBot @AzureAaron @LifeIsAParadox