Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(StartingInventory): Overhaul system; Improve documentation #13

Merged
merged 6 commits into from
May 25, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
*/
public class StartingInventoryComponent implements Component {

/**
* The list of objects contained in the starting inventory.
* <p>
* Default is an empty list.
*/
public List<InventoryItem> items = Lists.newLinkedList();

/**
Expand All @@ -66,6 +71,8 @@ public static class InventoryItem {

/**
* Must be greater than 0.
* <p>
* Default value is 1.
*/
public int quantity = 1;

Expand All @@ -74,6 +81,8 @@ public static class InventoryItem {
* <p>
* Adding inventory items to this list will cause a {@link InventoryComponent} to be added to this object. The
* nested inventory is filled with the items specified in this list.
* <p>
* Default value is the empty list.
*/
public List<InventoryItem> items = Lists.newLinkedList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.terasology.logic.inventory;

import com.google.common.collect.Lists;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.entitySystem.entity.EntityManager;
Expand All @@ -32,6 +31,7 @@

import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -62,18 +62,8 @@ public void initialise() {
@ReceiveEvent
public void onStartingInventory(OnPlayerSpawnedEvent event,
EntityRef entityRef,
StartingInventoryComponent startingInventoryComponent) {

InventoryComponent inventoryComponent =
Optional.ofNullable(entityRef.getComponent(InventoryComponent.class))
.orElse(new InventoryComponent(40));
entityRef.addOrSaveComponent(inventoryComponent);

for (StartingInventoryComponent.InventoryItem item : startingInventoryComponent.items) {
if (validateItem(item)) {
addToInventory(entityRef, item, inventoryComponent);
}
}
StartingInventoryComponent startingInventory) {
addItemsIfPossible(entityRef, startingInventory.items);
entityRef.removeComponent(StartingInventoryComponent.class);
}

Expand All @@ -85,7 +75,7 @@ public void onStartingInventory(OnPlayerSpawnedEvent event,
* @param item the inventory item to validate
* @return true if the item has non-empty URI and quantity greater zero, false otherwise
*/
private boolean validateItem(StartingInventoryComponent.InventoryItem item) {
private boolean isValid(StartingInventoryComponent.InventoryItem item) {
if (item.uri == null || item.uri.isEmpty()) {
logger.warn("Improperly specified starting inventory item: Uri is null");
return false;
Expand All @@ -99,58 +89,66 @@ private boolean validateItem(StartingInventoryComponent.InventoryItem item) {
}

private void addToInventory(EntityRef entityRef,
StartingInventoryComponent.InventoryItem item,
InventoryComponent inventoryComponent) {
String uri = item.uri;
int quantity = item.quantity;

final List<EntityRef> objects =
tryAsBlock(uri, quantity, item.items)
.map(Optional::of)
.orElseGet(() -> tryAsItem(uri, quantity))
.orElse(Lists.newArrayList());

objects.forEach(o ->
inventoryManager.giveItem(entityRef, EntityRef.NULL, o)
);
StartingInventoryComponent.InventoryItem item) {

Optional<Supplier<EntityRef>> possibleItem =
resolveAsBlock(item).map(Optional::of).orElseGet(() -> resolveAsItem(item));
Copy link
Contributor Author

@skaldarnar skaldarnar May 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this understandable? Should we change this? Here are some alternatives:

Suggested change
Optional<Supplier<EntityRef>> possibleItem =
resolveAsBlock(item).map(Optional::of).orElseGet(() -> resolveAsItem(item));
Optional<Supplier<EntityRef>> possibleItem = resolveAsBlock(item);
if (!possibleItem.isPresent()) {
possibleItem = resolveAsItem(item);
}
if (!possibleItem.isPresent()) {
// use the item as it is present
}
Suggested change
Optional<Supplier<EntityRef>> possibleItem =
resolveAsBlock(item).map(Optional::of).orElseGet(() -> resolveAsItem(item));
final Supplier<EntityRef> possibleItem =
resolveAsBlock(item).orElse(resolveAsItem(item).orElse(null));
if (possibleItem != null) {
// use the item as it is present
}

Copy link
Contributor Author

@skaldarnar skaldarnar May 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Optional::or (Java 9) mentioned in the comments this part would become:

final Optional<Supplier<EntityRef>> possibleItem = 
    resolveAsBlock(item.uri).or(() -> resolveAsItem(item.uri))


possibleItem.ifPresent(itemCreator -> {
Stream.generate(itemCreator)
.limit(item.quantity)
.peek(i -> addItemsIfPossible(i, item.items))
.collect(Collectors.toList())
.forEach(o -> inventoryManager.giveItem(entityRef, EntityRef.NULL, o));
});
}

/**
* Adds an {@link InventoryComponent} to the given entity holding the specified items.
* Adds all valid objects to this entity if it has an item component.
* <p>
* Inventory objects are valid if {@link #isValid(StartingInventoryComponent.InventoryItem)} holds.
* <p>
* If the list of nested items is empty or the entity does not have an inventory component this method does
* nothing.
*
* @param entity the entity that should hold the nested inventory
* @param items items to be filled into the nested inventory
* @param entity the entity to add the starting inventory objects to
* @param items the objects to add to the entity's inventory
*/
private void addNestedInventory(EntityRef entity,
private void addItemsIfPossible(EntityRef entity,
List<StartingInventoryComponent.InventoryItem> items) {
InventoryComponent nestedInventory =
Optional.ofNullable(entity.getComponent(InventoryComponent.class))
.orElseGet(() -> new InventoryComponent(30));
entity.addOrSaveComponent(nestedInventory);
items.stream()
.filter(this::validateItem)
.forEach(i -> addToInventory(entity, i, nestedInventory));
if (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());
}
}

private Optional<List<EntityRef>> tryAsBlock(String uri,
int quantity,
List<StartingInventoryComponent.InventoryItem> nestedItems) {
return Optional.ofNullable(blockManager.getBlockFamily(uri))
.map(blockFamily ->
Stream.generate(() -> blockFactory.newInstance(blockFamily))
.limit(quantity)
.peek(block -> {
if (!nestedItems.isEmpty()) {
addNestedInventory(block, nestedItems);
}
})
.collect(Collectors.toList()));
/**
* Attempt to resolve the given item as block and yield a supplier for new block items.
*
* @param object the item to resolve as block item
* @return an optional supplier for block items if the item references a block family, empty otherwise
*/
private Optional<Supplier<EntityRef>> resolveAsBlock(StartingInventoryComponent.InventoryItem object) {
return Optional.ofNullable(blockManager.getBlockFamily(object.uri))
.map(blockFamily -> () -> blockFactory.newInstance(blockFamily));
}

private Optional<List<EntityRef>> tryAsItem(String uri,
int quantity) {
return Optional.ofNullable(prefabManager.getPrefab(uri))
.filter(p -> p.hasComponent(ItemComponent.class))
.map(p -> Stream.generate(() -> entityManager.create(uri)).limit(quantity).collect(Collectors.toList()));
/**
* Attempt to resolve the given item as item prefab and yield a supplier to create new item instances.
* <p>
* The prefab the object URI resolves to must have an {@link ItemComponent}.
*
* @param object the item to resolve as item prefab
* @return an optional supplier for a prefab item if the object URI resolves to a prefab, empty otherwise
*/
private Optional<Supplier<EntityRef>> resolveAsItem(StartingInventoryComponent.InventoryItem object) {
return Optional.ofNullable(prefabManager.getPrefab(object.uri))
.filter(prefab -> prefab.hasComponent(ItemComponent.class))
.map(prefab -> () -> entityManager.create(object.uri));
}
}