From e4583f3ccc15680a550d941506a399391452a685 Mon Sep 17 00:00:00 2001 From: Tobias Nett Date: Sat, 30 May 2020 16:41:26 +0200 Subject: [PATCH] refactor(StartingInventory): Collect errors and log at the end (#18) * refactor(StartingInventory): Collect errors and log at the end * chore: Adjust module patch version (instead of minor) * fix(StartingInventory): Resolve item before block Co-authored-by: jdrueckert --- module.txt | 2 +- .../inventory/StartingInventorySystem.java | 60 +++++++++++++++---- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/module.txt b/module.txt index 56c8a64941..444830477f 100644 --- a/module.txt +++ b/module.txt @@ -1,6 +1,6 @@ { "id" : "Inventory", - "version" : "1.3.0", + "version" : "1.2.1", "isReleaseManaged": true, "displayName": "Core Inventory", "author": "The Terasology Foundation", diff --git a/src/main/java/org/terasology/logic/inventory/StartingInventorySystem.java b/src/main/java/org/terasology/logic/inventory/StartingInventorySystem.java index b403424181..9714d620c6 100644 --- a/src/main/java/org/terasology/logic/inventory/StartingInventorySystem.java +++ b/src/main/java/org/terasology/logic/inventory/StartingInventorySystem.java @@ -15,6 +15,7 @@ */ package org.terasology.logic.inventory; +import com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.terasology.entitySystem.entity.EntityManager; @@ -31,6 +32,7 @@ import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Stream; @@ -53,6 +55,11 @@ public class StartingInventorySystem extends BaseComponentSystem { BlockItemFactory blockFactory; + /** + * Collect entities without inventory component, which have nested items configured in the {@link StartingInventoryComponent}. + */ + private final Set entitiesWithoutInventory = Sets.newHashSet(); + @Override public void initialise() { blockFactory = new BlockItemFactory(entityManager); @@ -60,10 +67,24 @@ public void initialise() { @ReceiveEvent public void onStartingInventory(OnPlayerSpawnedEvent event, - EntityRef entityRef, + EntityRef player, StartingInventoryComponent startingInventory) { - addItemsTo(startingInventory.items, entityRef); - entityRef.removeComponent(StartingInventoryComponent.class); + entitiesWithoutInventory.clear(); + addItemsTo(startingInventory.items, player, player.getParentPrefab().getName()); + player.removeComponent(StartingInventoryComponent.class); + logErrors(); + } + + /** + * Collectively log errors found during the assembly of the starting inventory. + *

+ * The collected errors in {@link #entitiesWithoutInventory} will be empty after this call returns. + */ + private void logErrors() { + if (!entitiesWithoutInventory.isEmpty()) { + logger.warn("Cannot add starting inventory objects to entities without inventory component!\n{}", entitiesWithoutInventory); + entitiesWithoutInventory.clear(); + } } /** @@ -87,20 +108,31 @@ private boolean isValid(StartingInventoryComponent.InventoryItem item) { return true; } - private void addToInventory(EntityRef entityRef, + /** + * Attempt to resolve the given {@code item} as block or item and add it to the entity's inventory. + *

+ * If the item cannot be resolved the target inventory will not be changed. + *

+ * Calling this method may add errors to {@link #entitiesWithoutInventory}. + * + * @param entity the entity with {@link InventoryComponent} to add the item to + * @param item the item to add to the entity's inventory + */ + private void addToInventory(EntityRef entity, StartingInventoryComponent.InventoryItem item) { - //TODO(Java9): Use Optional::or instead (https://docs.oracle.com/javase/9/docs/api/java/util/Optional.html#or-java.util.function.Supplier-) + //TODO(Java9): Use Optional::or instead (https://docs.oracle.com/javase/9/docs/api/java/util/Optional + // .html#or-java.util.function.Supplier-) // This article describes the issue and provides the solution used here: // https://www.baeldung.com/java-optional-or-else-optional#1-lazy-evaluation final Optional> possibleItem = - resolveAsBlock(item.uri).map(Optional::of).orElseGet(() -> resolveAsItem(item.uri)); + resolveAsItem(item.uri).map(Optional::of).orElseGet(() -> resolveAsBlock(item.uri)); if (possibleItem.isPresent()) { Stream.generate(possibleItem.get()) .limit(item.quantity) - .map(i -> addItemsTo(item.items, i)) - .forEach(o -> inventoryManager.giveItem(entityRef, EntityRef.NULL, o)); + .map(i -> addItemsTo(item.items, i, item.uri)) + .forEach(o -> inventoryManager.giveItem(entity, EntityRef.NULL, o)); } else { logger.warn("Could not resolve '{}' to either block or item.", item.uri); } @@ -113,19 +145,21 @@ private void addToInventory(EntityRef entityRef, *

* If the list of nested items is empty or the entity does not have an inventory component this method does * nothing. + *

+ * Calling this method may add errors to {@link #entitiesWithoutInventory}. * * @param items the objects to add to the entity's inventory * @param entity the entity to add the starting inventory objects to + * @param entityDescriptor a descriptive string (name or Uri) for the entity, used for logging */ - private EntityRef addItemsTo(List items, EntityRef entity) { - if (entity.hasComponent(InventoryComponent.class)) { + private EntityRef addItemsTo(List items, EntityRef entity, + String entityDescriptor) { + if (items.isEmpty() || entity.hasComponent(InventoryComponent.class)) { items.stream() .filter(this::isValid) .forEach(item -> addToInventory(entity, item)); } else { - logger.warn( - "Cannot add starting inventory objects to entity without inventory component!\n{}", - entity.toFullDescription()); + entitiesWithoutInventory.add(entityDescriptor); } return entity; }