Skip to content

Commit

Permalink
Limit number of Loki ring offsets and prevent cost calculation overflow
Browse files Browse the repository at this point in the history
Fixes #4681 by restricting the number of offsets to prevent the ring's NBT data from exceeding protocol limits. This also contains a fix to a potential integer overflow in the mana calculation that can happen within intended use of the ring already.
  • Loading branch information
TheRealWormbo committed Jun 20, 2024
1 parent 7b0ec1d commit a7fbc99
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import net.minecraft.core.BlockPos;
import net.minecraft.core.Direction;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.network.chat.Component;
import net.minecraft.server.level.ServerPlayer;
import net.minecraft.util.Mth;
import net.minecraft.world.InteractionHand;
Expand Down Expand Up @@ -52,6 +53,11 @@

public class RingOfLokiItem extends RelicBaubleItem implements WireframeCoordinateListProvider {

/**
* This limit exists to prevent players from accidentally NBT-banning themselves from a world or server.
* TODO 1.21: It might be possible to increase this if the storage tag structure is optimized.
*/
private static final int MAX_NUM_CURSORS = 1023;
private static final String TAG_CURSOR_LIST = "cursorList";
private static final String TAG_CURSOR_PREFIX = "cursor";
private static final String TAG_CURSOR_COUNT = "cursorCount";
Expand Down Expand Up @@ -99,7 +105,11 @@ public static InteractionResult onPlayerInteract(Player player, Level world, Int

boolean removed = cursors.remove(relPos);
if (!removed) {
cursors.add(relPos);
if (cursors.size() < MAX_NUM_CURSORS) {
cursors.add(relPos);
} else {
player.displayClientMessage(Component.translatable("botaniamisc.lokiRingLimitReached"), true);
}
}
setCursorList(lokiRing, cursors);
}
Expand All @@ -108,7 +118,9 @@ public static InteractionResult onPlayerInteract(Player player, Level world, Int

return InteractionResult.SUCCESS;
} else {
int cost = Math.min(cursors.size(), (int) Math.pow(Math.E, cursors.size() * 0.25));
int numCursors = cursors.size();
// particularly large cursor counts can overflow after exponentiation
int cost = numCursors > 10 ? numCursors : Math.min(numCursors, (int) Math.pow(Math.E, numCursors * 0.25));
ItemStack original = stack.copy();
int successes = 0;
for (BlockPos cursor : cursors) {
Expand Down
1 change: 1 addition & 0 deletions Xplat/src/main/resources/assets/botania/lang/en_us.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
"botaniamisc.invalidDodge": "Invalid Dodge Packet",
"botaniamisc.rannuncarpus.state_sensitive": "Match Exact State",
"botaniamisc.rannuncarpus.state_insensitive": "Match Block Only",
"botaniamisc.lokiRingLimitReached": "Selection limit reached",

"botania.tater_birthday.0": "Wow, is this for me?",
"botania.tater_birthday.1": "It's my birthday today; I'm %d years old now!",
Expand Down
2 changes: 2 additions & 0 deletions web/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ and start a new "Upcoming" section.
* Missing lookup references for derivatives of certain decorative blocks (e.g. quartz variants) have been added for the quick-lookup feature while holding the book
* Lexica Botania acknowledges that Trinkets are used on Fabric instead of Curios
* Various entries were updated to match recent changes and fix old errors
* Change: The Ring of Loki now has a (reasonably high) selection limit to prevent players from accidentally "NBT-banning" themselves from their world or server by going too hard with it
* Remove: Config option `orechidPriorityMods` didn't have any effect since 1.16, never made it to Fabric, and is now gone in the Forge version as well; pack authors should use data pack recipes to define Orechid outputs and weights
* Fix: Flight bar for Flügel Tiara no longer overlaps with the refilling air bubbles indicator or the mount health bar, if that uses more than one row
* Fix: The Manaseer Monocle's flower radius indicator no longer jumps around if you are very far from the world origin, and should also not Z-fight with the binding radius indicator of luminizers anymore
Expand All @@ -47,6 +48,7 @@ and start a new "Upcoming" section.
* Fix: Floating flower islands properly show the soil type on Forge, and are no longer invisible in Patchouli multiblock visualizations (although that latter part is merely a workaround for a Patchouli issue at the moment)
* Fix: Position of the binding information icon on flowers' wand HUD has been adjusted to not overlap with longer flower names
* Fix: Suspicious stew effect from Pure Daisy actually works now, clearing all active status effects
* Fix: Potential integer overflow in Ring of Loki mana calculation

---

Expand Down

0 comments on commit a7fbc99

Please sign in to comment.