Paper icon indicating copy to clipboard operation
Paper copied to clipboard

Expose saturation in FoodLevelChangeEvent

Open Owen1212055 opened this issue 4 years ago • 7 comments

Resolves https://github.com/PaperMC/Paper/issues/7193

It may just be better to add an entirely new event.. but I would like opinions on this. Although this mostly works fine, it gets a little wonky when it comes to canceling natural hunger decay.

((ServerPlayer) this.entityhuman).getBukkitEntity().sendHealthUpdate();
@@ -66,10 +66,11 @@ public class FoodData {
                 this.saturationLevel = Math.max(this.saturationLevel - 1.0F, 0.0F);
             } else if (enumdifficulty != Difficulty.PEACEFUL) {
                 // CraftBukkit start
-                org.bukkit.event.entity.FoodLevelChangeEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callFoodLevelChangeEvent(player, Math.max(this.foodLevel - 1, 0));
+                org.bukkit.event.entity.FoodLevelChangeEvent event = org.bukkit.craftbukkit.event.CraftEventFactory.callFoodLevelChangeEvent(player, Math.max(this.foodLevel - 1, 0), this.saturationLevel); // Paper - Expose Saturation

                 if (!event.isCancelled()) {
                     this.foodLevel = event.getFoodLevel();
+                    this.saturationLevel = event.getSaturationModifier(); // Paper - Expose Saturation
                 }

Specifically here, I can't really "cancel" saturation modification since this would be a behavior change. So, it's either we can live with this slight issue, or we can just make an entirely new event that would be executed alongside FoodLevelChangeEvent, this would allow us to add an event for natural saturation decay as well. However, canceling the FoodLevelChangeEvent will also cancel modification of the saturation in some situations (like eating food). So I would like some pointers here.

Owen1212055 avatar Dec 26 '21 04:12 Owen1212055

Alright, I now capture the result and store it instead of trying to manually do the logic (except for the tick methods). This also causes a fix as a result, where the food result would appear greater than 20 since there was no logic for making sure it was less than 20 in the event, but there was in the eat method.

I also made saturation represent the new changes saturation to fit food level.

Owen1212055 avatar Dec 27 '21 00:12 Owen1212055

I guess that it makes more sense to represent the "new saturation level" instead of the modifier, since the food level also provides the final value and not the delta. The only "problem" I have with it, is that in my opinion it kind of misses the point of my initial issue, at least regarding what I was trying to achieve. Having the final value might be useful in some cases, but it is less useful for modifying food, since one has to do the saturation calculation manually according to the modified food level and still has to figure out the correct formula. The only issue that having access to the value would solve, is somehow setting the new saturation after the event has processed. But maybe this event isn't ideal for implementing custom food, and a new event for eating would be the better way to go (That could also provide the food delta and the modifier, instead of final values).

aerulion avatar Dec 28 '21 12:12 aerulion

I guess that it makes more sense to represent the "new saturation level" instead of the modifier, since the food level also provides the final value and not the delta. The only "problem" I have with it, is that in my opinion it kind of misses the point of my initial issue, at least regarding what I was trying to achieve. Having the final value might be useful in some cases, but it is less useful for modifying food, since one has to do the saturation calculation manually according to the modified food level and still has to figure out the correct formula. The only issue that having access to the value would solve, is somehow setting the new saturation after the event has processed. But maybe this event isn't ideal for implementing custom food, and a new event for eating would be the better way to go (That could also provide the food delta and the modifier, instead of final values).

Agreed! Added a separate event for this, https://github.com/PaperMC/Paper/pull/7213

Owen1212055 avatar Dec 28 '21 17:12 Owen1212055

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Feb 27 '22 04:02 stale[bot]

This shouldn't be closed as I desire this feature

nathanfranke avatar Mar 12 '22 17:03 nathanfranke

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 06 '22 22:07 stale[bot]

bruh

nathanfranke avatar Jul 06 '22 22:07 nathanfranke