search

SpongePowered / Sponge

The SpongeAPI implementation targeting vanilla Minecraft and 3rd party platforms.
MIT License
383 stars 210 forks source link

Remove Raid#setBossBar() #4078

Closed MrHell228 closed 1 month ago

MrHell228 commented 2 months ago

SpongeAPI | Sponge

Faithcaio commented 2 months ago

There is code in place that should make this work https://github.com/SpongePowered/Sponge/blob/api-12/src/mixins/java/org/spongepowered/common/mixin/core/adventure/bossbar/HackyBossBarPlatformBridgeMixin.java not sure what the use case for it was/is

MrHell228 commented 2 months ago

Make it working somehow is not really a problem, that was just a little fun fact. The whole point of adventure BossBar is that it's mutable, so no setting is required. I can't think of actual use case where #bossBar() is not enough and set method is neccesary. Changing ServerBossEvent field with new boss bar can at least lead to potential data loss (e.g. if mod adds its own field to boss bar and we completely ignore it). We also don't support setting Keys.BOSS_BAR so I think here we should not provide setting as well

MrHell228 commented 1 month ago

Actually found why it's there. It was added before adventure was there and seems like sponge boss bar wasn't mutable. So I think this is just legacy method now. https://github.com/SpongePowered/Sponge/commit/80628b84b93184bc6dafb16b85465ca588afb0a0#diff-1305855de850c8a05491e231159bceb83da9a3a05ca72cff587052cb74f5bb4bR71-R74

Faithcaio commented 1 month ago

https://github.com/SpongePowered/Sponge/commit/dbde30713fa95763979a859ae5b851de4e36b80a