Improve transfer API debug messages (#3049)

* Add toStrings to Storage, ContainerItemContext and Transaction impls

* StorageUtil: Use crash reports for adding context to errors

* Drop Impl suffix from variant impl toStrings

Since they are the only implementations, it offers no meaningful info for
callers.

* Centralise formatting code, include BE inventory position

* Include amount and resource in all ContainerItemContext.toString impls

* Use crash callables in StorageUtil

* Add crash report to FluidStorageUtil

* Add owning thread to TransactionImpl.toString

* Use thread name in TransactionImpl.toString

The other info clutters the message.

* Fix code style
This commit is contained in:
Juuz 2023-05-14 16:56:35 +03:00 committed by modmuss50
parent dcb9d1ca7f
commit e78ef151ef
29 changed files with 295 additions and 21 deletions

View file

@ -16,6 +16,8 @@
package net.fabricmc.fabric.api.transfer.v1.fluid;
import java.util.Objects;
import org.jetbrains.annotations.ApiStatus;
import net.minecraft.entity.player.PlayerEntity;
@ -26,11 +28,14 @@ import net.minecraft.sound.SoundCategory;
import net.minecraft.sound.SoundEvent;
import net.minecraft.sound.SoundEvents;
import net.minecraft.util.Hand;
import net.minecraft.util.crash.CrashException;
import net.minecraft.util.crash.CrashReport;
import net.fabricmc.fabric.api.transfer.v1.context.ContainerItemContext;
import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
import net.fabricmc.fabric.api.transfer.v1.storage.StorageView;
import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* Helper functions to work with fluid storages.
@ -63,7 +68,18 @@ public final class FluidStorageUtil {
// Try to fill hand first, otherwise try to empty it.
Item handItem = player.getStackInHand(hand).getItem();
return moveWithSound(storage, handStorage, player, true, handItem) || moveWithSound(handStorage, storage, player, false, handItem);
try {
return moveWithSound(storage, handStorage, player, true, handItem) || moveWithSound(handStorage, storage, player, false, handItem);
} catch (Exception e) {
CrashReport report = CrashReport.create(e, "Interacting with fluid storage");
report.addElement("Interaction details")
.add("Player", () -> DebugMessages.forPlayer(player))
.add("Hand", hand)
.add("Hand item", handItem::toString)
.add("Fluid storage", () -> Objects.toString(storage, null));
throw new CrashException(report);
}
}
private static boolean moveWithSound(Storage<FluidVariant> from, Storage<FluidVariant> to, PlayerEntity player, boolean fill, Item handItem) {

View file

@ -127,4 +127,10 @@ public final class EmptyItemFluidStorage implements InsertionOnlyStorage<FluidVa
public Iterator<StorageView<FluidVariant>> iterator() {
return blankView.iterator();
}
@Override
public String toString() {
return "EmptyItemFluidStorage[context=%s, insertableFluid=%s, insertableAmount=%d]"
.formatted(context, insertableFluid, insertableAmount);
}
}

View file

@ -132,4 +132,10 @@ public final class FullItemFluidStorage implements ExtractionOnlyStorage<FluidVa
// Capacity is the same as the amount.
return getAmount();
}
@Override
public String toString() {
return "FullItemFluidStorage[context=%s, fluid=%s, amount=%d]"
.formatted(context, containedFluid, containedAmount);
}
}

View file

@ -164,4 +164,9 @@ public abstract class SingleStackStorage extends SnapshotParticipant<ItemStack>
protected void readSnapshot(ItemStack snapshot) {
setStack(snapshot);
}
@Override
public String toString() {
return "SingleStackStorage[" + getStack() + "]";
}
}

View file

@ -25,6 +25,8 @@ import org.jetbrains.annotations.Nullable;
import net.minecraft.inventory.Inventory;
import net.minecraft.screen.ScreenHandler;
import net.minecraft.util.crash.CrashException;
import net.minecraft.util.crash.CrashReport;
import net.minecraft.util.math.MathHelper;
import net.fabricmc.fabric.api.transfer.v1.storage.base.ResourceAmount;
@ -118,6 +120,15 @@ public final class StorageUtil {
}
iterationTransaction.commit();
} catch (Exception e) {
CrashReport report = CrashReport.create(e, "Moving resources between storages");
report.addElement("Move details")
.add("Input storage", from::toString)
.add("Output storage", to::toString)
.add("Filter", filter::toString)
.add("Max amount", maxAmount)
.add("Transaction", transaction);
throw new CrashException(report);
}
return totalMoved;
@ -140,10 +151,19 @@ public final class StorageUtil {
if (storage == null) return null;
for (StorageView<T> view : storage.nonEmptyViews()) {
T resource = view.getResource();
long amount = view.extract(resource, maxAmount, transaction);
if (amount > 0) return new ResourceAmount<>(resource, amount);
try {
for (StorageView<T> view : storage.nonEmptyViews()) {
T resource = view.getResource();
long amount = view.extract(resource, maxAmount, transaction);
if (amount > 0) return new ResourceAmount<>(resource, amount);
}
} catch (Exception e) {
CrashReport report = CrashReport.create(e, "Extracting resources from storage");
report.addElement("Extraction details")
.add("Storage", storage::toString)
.add("Max amount", maxAmount)
.add("Transaction", transaction);
throw new CrashException(report);
}
return null;
@ -160,16 +180,26 @@ public final class StorageUtil {
StoragePreconditions.notNegative(maxAmount);
long amount = 0;
for (SingleSlotStorage<T> slot : slots) {
if (!slot.isResourceBlank()) {
try {
for (SingleSlotStorage<T> slot : slots) {
if (!slot.isResourceBlank()) {
amount += slot.insert(resource, maxAmount - amount, transaction);
if (amount == maxAmount) return amount;
}
}
for (SingleSlotStorage<T> slot : slots) {
amount += slot.insert(resource, maxAmount - amount, transaction);
if (amount == maxAmount) return amount;
}
}
for (SingleSlotStorage<T> slot : slots) {
amount += slot.insert(resource, maxAmount - amount, transaction);
if (amount == maxAmount) return amount;
} catch (Exception e) {
CrashReport report = CrashReport.create(e, "Inserting resources into slots");
report.addElement("Slotted insertion details")
.add("Slots", () -> Objects.toString(slots, null))
.add("Resource", () -> Objects.toString(resource, null))
.add("Max amount", maxAmount)
.add("Transaction", transaction);
throw new CrashException(report);
}
return amount;
@ -187,12 +217,22 @@ public final class StorageUtil {
public static <T> long tryInsertStacking(@Nullable Storage<T> storage, T resource, long maxAmount, TransactionContext transaction) {
StoragePreconditions.notNegative(maxAmount);
if (storage instanceof SlottedStorage<T> slottedStorage) {
return insertStacking(slottedStorage.getSlots(), resource, maxAmount, transaction);
} else if (storage != null) {
return storage.insert(resource, maxAmount, transaction);
} else {
return 0;
try {
if (storage instanceof SlottedStorage<T> slottedStorage) {
return insertStacking(slottedStorage.getSlots(), resource, maxAmount, transaction);
} else if (storage != null) {
return storage.insert(resource, maxAmount, transaction);
} else {
return 0;
}
} catch (Exception e) {
CrashReport report = CrashReport.create(e, "Inserting resources into a storage");
report.addElement("Insertion details")
.add("Storage", () -> Objects.toString(storage, null))
.add("Resource", () -> Objects.toString(resource, null))
.add("Max amount", maxAmount)
.add("Transaction", transaction);
throw new CrashException(report);
}
}

View file

@ -17,6 +17,7 @@
package net.fabricmc.fabric.api.transfer.v1.storage.base;
import java.util.List;
import java.util.StringJoiner;
import org.jetbrains.annotations.ApiStatus;
@ -64,4 +65,15 @@ public class CombinedSlottedStorage<T, S extends SlottedStorage<T>> extends Comb
throw new IndexOutOfBoundsException("Slot " + slot + " is out of bounds. This storage has size " + getSlotCount());
}
@Override
public String toString() {
StringJoiner partNames = new StringJoiner(", ");
for (S part : parts) {
partNames.add(part.toString());
}
return "CombinedSlottedStorage[" + partNames + "]";
}
}

View file

@ -19,6 +19,7 @@ package net.fabricmc.fabric.api.transfer.v1.storage.base;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.StringJoiner;
import org.jetbrains.annotations.ApiStatus;
@ -99,6 +100,17 @@ public class CombinedStorage<T, S extends Storage<T>> implements Storage<T> {
return new CombinedIterator();
}
@Override
public String toString() {
StringJoiner partNames = new StringJoiner(", ");
for (S part : parts) {
partNames.add(part.toString());
}
return "CombinedStorage[" + partNames + "]";
}
/**
* The combined iterator for multiple storages.
*/

View file

@ -182,6 +182,11 @@ public abstract class FilteringStorage<T> implements Storage<T> {
return backingStorage.get().getVersion();
}
@Override
public String toString() {
return "FilteringStorage[" + backingStorage.get() + "/" + backingStorage + "]";
}
/**
* This is used to ensure extractions through storage views of the backing stored also get checked by {@link #canExtract}.
*/

View file

@ -222,4 +222,9 @@ public abstract class SingleVariantItemStorage<T extends TransferVariant<?>> imp
return 0;
}
}
@Override
public String toString() {
return "SingleVariantItemStorage[" + context + "/" + item + "]";
}
}

View file

@ -158,4 +158,9 @@ public abstract class SingleVariantStorage<T extends TransferVariant<?>> extends
variant = snapshot.resource();
amount = snapshot.amount();
}
@Override
public String toString() {
return "SingleVariantStorage[%d %s]".formatted(amount, variant);
}
}

View file

@ -0,0 +1,53 @@
/*
* Copyright (c) 2016, 2017, 2018, 2019 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package net.fabricmc.fabric.impl.transfer;
import org.jetbrains.annotations.Nullable;
import net.minecraft.block.entity.BlockEntity;
import net.minecraft.entity.player.PlayerEntity;
import net.minecraft.entity.player.PlayerInventory;
import net.minecraft.inventory.Inventory;
import net.minecraft.util.math.BlockPos;
import net.minecraft.world.World;
public final class DebugMessages {
public static String forGlobalPos(@Nullable World world, BlockPos pos) {
String dimension = world != null ? world.getDimensionKey().getValue().toString() : "<no dimension>";
return dimension + "@" + pos.toShortString();
}
public static String forPlayer(PlayerEntity player) {
return player.getEntityName() + "/" + player.getUuidAsString();
}
public static String forInventory(@Nullable Inventory inventory) {
if (inventory == null) {
return "~~NULL~~"; // like in crash reports
} else if (inventory instanceof PlayerInventory playerInventory) {
return forPlayer(playerInventory.player);
} else {
String result = inventory.toString();
if (inventory instanceof BlockEntity blockEntity) {
result += " (%s, %s)".formatted(blockEntity.getCachedState(), forGlobalPos(blockEntity.getWorld(), blockEntity.getPos()));
}
return result;
}
}
}

View file

@ -66,6 +66,11 @@ public class TransferApiImpl {
public long getVersion() {
return 0;
}
@Override
public String toString() {
return "EmptyStorage";
}
};
public static <T> Iterator<T> singletonIterator(T it) {

View file

@ -76,4 +76,10 @@ public class ConstantContainerItemContext implements ContainerItemContext {
public List<SingleSlotStorage<ItemVariant>> getAdditionalSlots() {
return Collections.emptyList();
}
@Override
public String toString() {
return "ConstantContainerItemContext[%d %s]"
.formatted(getMainSlot().getAmount(), getMainSlot().getResource());
}
}

View file

@ -56,4 +56,10 @@ public class CreativeInteractionContainerItemContext extends ConstantContainerIt
// Insertion always succeeds from the POV of the context user.
return maxAmount;
}
@Override
public String toString() {
return "CreativeInteractionContainerItemContext[%d %s]"
.formatted(getMainSlot().getAmount(), getMainSlot().getResource());
}
}

View file

@ -56,4 +56,10 @@ public class PlayerContainerItemContext implements ContainerItemContext {
public List<SingleSlotStorage<ItemVariant>> getAdditionalSlots() {
return playerWrapper.getSlots();
}
@Override
public String toString() {
return "PlayerContainerItemContext[%d %s %s/%s]"
.formatted(slot.getAmount(), slot.getResource(), playerWrapper, slot);
}
}

View file

@ -46,4 +46,10 @@ public class SingleSlotContainerItemContext implements ContainerItemContext {
public List<SingleSlotStorage<ItemVariant>> getAdditionalSlots() {
return Collections.emptyList();
}
@Override
public String toString() {
return "SingleSlotContainerItemContext[%d %s %s]"
.formatted(slot.getAmount(), slot.getResource(), slot);
}
}

View file

@ -32,6 +32,7 @@ import net.fabricmc.fabric.api.transfer.v1.storage.StoragePreconditions;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* Standard implementation of {@code Storage<FluidVariant>}, using cauldron/fluid mappings registered in {@link CauldronFluidContent}.
@ -47,6 +48,10 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
public class CauldronStorage extends SnapshotParticipant<BlockState> implements SingleSlotStorage<FluidVariant> {
// Record is used for convenient constructor, hashcode and equals implementations.
private record WorldLocation(World world, BlockPos pos) {
@Override
public String toString() {
return DebugMessages.forGlobalPos(world, pos);
}
}
// Weak values to make sure wrappers are cleaned up after use, thread-safe.
@ -204,4 +209,9 @@ public class CauldronStorage extends SnapshotParticipant<BlockState> implements
location.world.setBlockState(location.pos, state);
}
}
@Override
public String toString() {
return "CauldronStorage[" + location + "]";
}
}

View file

@ -70,4 +70,9 @@ public class EmptyBucketStorage implements InsertionOnlyStorage<FluidVariant> {
public Iterator<StorageView<FluidVariant>> iterator() {
return blankView.iterator();
}
@Override
public String toString() {
return "EmptyBucketStorage[" + context + "]";
}
}

View file

@ -131,7 +131,7 @@ public class FluidVariantImpl implements FluidVariant {
@Override
public String toString() {
return "FluidVariantImpl{fluid=" + fluid + ", tag=" + nbt + '}';
return "FluidVariant{fluid=" + fluid + ", tag=" + nbt + '}';
}
@Override

View file

@ -114,4 +114,9 @@ public class WaterPotionStorage implements ExtractionOnlyStorage<FluidVariant>,
// Capacity is the same as the amount.
return getAmount();
}
@Override
public String toString() {
return "WaterPotionStorage[" + context + "]";
}
}

View file

@ -41,6 +41,7 @@ import net.fabricmc.fabric.api.transfer.v1.storage.base.InsertionOnlyStorage;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* Implementation of {@code Storage<ItemVariant>} for composters.
@ -55,6 +56,11 @@ public class ComposterWrapper extends SnapshotParticipant<Float> {
private void setBlockState(BlockState state) {
world.setBlockState(pos, state);
}
@Override
public String toString() {
return DebugMessages.forGlobalPos(world, pos);
}
}
// Weak values to make sure wrappers are cleaned up after use, thread-safe.
@ -147,6 +153,11 @@ public class ComposterWrapper extends SnapshotParticipant<Float> {
increaseProbability = insertedIncreaseProbability;
return 1;
}
@Override
public String toString() {
return "ComposterWrapper[" + location + "/top]";
}
}
private class BottomStorage implements ExtractionOnlyStorage<ItemVariant>, SingleSlotStorage<ItemVariant> {
@ -192,5 +203,10 @@ public class ComposterWrapper extends SnapshotParticipant<Float> {
public long getCapacity() {
return 1;
}
@Override
public String toString() {
return "ComposterWrapper[" + location + "/bottom]";
}
}
}

View file

@ -21,6 +21,7 @@ import java.util.Map;
import com.google.common.collect.MapMaker;
import net.minecraft.item.ItemStack;
import net.minecraft.registry.Registries;
import net.minecraft.screen.ScreenHandler;
import net.fabricmc.fabric.api.transfer.v1.item.base.SingleStackStorage;
@ -50,4 +51,9 @@ public class CursorSlotWrapper extends SingleStackStorage {
protected void setStack(ItemStack stack) {
screenHandler.setCursorStack(stack);
}
@Override
public String toString() {
return "CursorSlotWrapper[" + screenHandler + "/" + Registries.SCREEN_HANDLER.getId(screenHandler.getType()) + "]";
}
}

View file

@ -26,9 +26,10 @@ import net.minecraft.item.ItemStack;
import net.minecraft.item.Items;
import net.minecraft.util.math.BlockPos;
import net.fabricmc.fabric.api.transfer.v1.item.base.SingleStackStorage;
import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.item.base.SingleStackStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* A wrapper around a single slot of an inventory.
@ -161,4 +162,9 @@ class InventorySlotWrapper extends SingleStackStorage {
original.setCount(0);
}
}
@Override
public String toString() {
return "InventorySlotWrapper[%s#%d]".formatted(DebugMessages.forInventory(storage.inventory), slot);
}
}

View file

@ -34,6 +34,7 @@ import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.storage.base.CombinedStorage;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* Implementation of {@link InventoryStorage}.
@ -112,6 +113,11 @@ public class InventoryStorageImpl extends CombinedStorage<ItemVariant, SingleSlo
}
}
@Override
public String toString() {
return "InventoryStorage[" + DebugMessages.forInventory(inventory) + "]";
}
// Boolean is used to prevent allocation. Null values are not allowed by SnapshotParticipant.
class MarkDirtyParticipant extends SnapshotParticipant<Boolean> {
@Override

View file

@ -122,7 +122,7 @@ public class ItemVariantImpl implements ItemVariant {
@Override
public String toString() {
return "ItemVariantImpl{item=" + item + ", tag=" + nbt + '}';
return "ItemVariant{item=" + item + ", tag=" + nbt + '}';
}
@Override

View file

@ -30,6 +30,7 @@ import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerInventoryStorage {
private final DroppedStacks droppedStacks;
@ -96,6 +97,11 @@ class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerI
}
}
@Override
public String toString() {
return "PlayerInventoryStorage[" + DebugMessages.forInventory(playerInventory) + "]";
}
private class DroppedStacks extends SnapshotParticipant<Integer> {
final List<Entry> entries = new ArrayList<>();

View file

@ -23,6 +23,7 @@ import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.storage.StorageView;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.impl.transfer.DebugMessages;
/**
* Wrapper around an {@link InventorySlotWrapper}, with additional canInsert and canExtract checks.
@ -80,4 +81,9 @@ class SidedInventorySlotWrapper implements SingleSlotStorage<ItemVariant> {
public StorageView<ItemVariant> getUnderlyingView() {
return slotWrapper.getUnderlyingView();
}
@Override
public String toString() {
return "SidedInventorySlotWrapper[%s#%d/%s]".formatted(DebugMessages.forInventory(sidedInventory), slotWrapper.slot, direction.getName());
}
}

View file

@ -32,8 +32,11 @@ import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
* Sidedness-aware wrapper around a {@link InventoryStorageImpl} for sided inventories.
*/
class SidedInventoryStorageImpl extends CombinedStorage<ItemVariant, SingleSlotStorage<ItemVariant>> implements InventoryStorage {
private final InventoryStorageImpl backingStorage;
SidedInventoryStorageImpl(InventoryStorageImpl storage, Direction direction) {
super(Collections.unmodifiableList(createWrapperList(storage, direction)));
this.backingStorage = storage;
}
@Override
@ -52,4 +55,10 @@ class SidedInventoryStorageImpl extends CombinedStorage<ItemVariant, SingleSlotS
return Arrays.asList(slots);
}
@Override
public String toString() {
// These two are the same from the user's perspective.
return backingStorage.toString();
}
}

View file

@ -235,5 +235,10 @@ public class TransactionManagerImpl {
outerCloseCallbacks.add(outerCloseCallback);
}
@Override
public String toString() {
return "Transaction[depth=%d, lifecycle=%s, thread=%s]".formatted(nestingDepth, lifecycle.name(), thread.getName());
}
}
}