Implement more inventory mocks

[fhnaumann]

See #1010 for more details.

Description

Barebone implementation for workbench inventory (tests and documentation missing).

My general idea is to use items in InventoryMock as the "storage contents" of the inventory. That means this array contains every slot where methods like addItems or removeItems have an effect. For example, adding (not setting) items to a workbench should only fill the crafting matrix but not the result slot. With this implementation this behaviour does not require any changes in InventoryMock, because from its point of view it only knows about the existence of the crafting matrix. Additional slots that have unique behaviour (as far as I know that only the result slot in different inventories), are handled individually in their respective mock implementation. This approach is somehwat similar to what Bukkit/Spigot/Paper does. They have a default "inventory" (rather its an abstraction layer above inventory) which for a crafting inventory would be the matrix. Then they have an additional "inventory" that represents the result slot.

Checklist

The following items should be checked before the pull request can be merged.

• Code follows existing style.
• Unit tests added (if applicable).

[Thorinwasher]

I'm mainly wondering if there could be better names for Resultable and Matrixable. I'm not the best at names here, but it's better to give the name a bit more context, MatrixableInventorySimulation maybe?

Also is this abstraction going to be used elsewhere than in WorkbenchSimulation? If not, then it's not really necesary

[fhnaumann]

I can change these names, maybe to IHasMatrixSlot and IHasResultSlot. I like how concise they are, but I hate annotation interface with I. The abstraction is going to be used in other inventory simulations. For example, the internal player crafting would implement both interfaces. Inventories like Furnace would only implement IHasResultSlot (and maybe something else to represent the fuel slot).

[Thorinwasher]

I can change these names, maybe to IHasMatrixSlot and IHasResultSlot. I like how concise they are, but I hate annotation interface with I. The abstraction is going to be used in other inventory simulations. For example, the internal player crafting would implement both interfaces. Inventories like Furnace would only implement IHasResultSlot (and maybe something else to represent the fuel slot).

Let's avoid that I notation. I don't see the issue with having long descriptive names here, now that think about it, my suggestion should be InventoryMatrixSimulation (or something like that) and InventoryResultSimulation.

[fhnaumann]

I had to change this because using type.getDefaultSize() would create arrays larger than necessary, as result slots are not in this array. This also caused a unit test to fail in InventoryMockTest because there getSize() is not implemented adequately.

[fhnaumann]

I was wrong with my initial comment. It seems like adding to an inventory can in fact also add it to the result slot. In this case items.length==9 (matrix) and getSize()==10 (workbench mock overrides and accounts the result slot). Subsequently calling setItem is necessary instead of directly assigning it to the array, because adding to the result slot has to be possible as well.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[fhnaumann]

To summarize the weird implementation of inventories with result slots:

• Some inventories extend CraftResultInventory (for example: Anvil)
  • These inventories store the result slot as a separate inventory and add items to the "correct" inventory based on the given index in setItem. They do not override getStorageContents or getContents and therefore searching, adding or removing (without an index, e.g. methods like firstEmpty or contains) completely ignores the result slot/inventory.
• Some inventories that have result slots do not extend CraftResultInventory. Notably Furnace is an example. In this case the result slot is accounted for in the search methods.
• CraftInventoryCrafting is an exception where it does not extend CraftResultInventory, has its own additional inventory for the result slot, but explicitly overrides getContents. Therefore the result slot in a workbench is accounted for in the search methods.

[fhnaumann]

Going to postpone this until paper makes inventories more consistent (see here).