organize under single `inventory` package

keturn commented 3 years ago

The package names currently used in this module make it difficult to distinguish which classes are in this module and which are from engine or elsewhere.

Recommend all packages here descend from a single org.terasology.inventory (or org.terasology.module.inventory?) instead of org.terasology.logic & rendering.

I'm singling some of these out not just because they're not under a single package, but also because health and inventory packages still exist in engine, making things extra muddled.

keturn commented 3 years ago

This is the current package organization under org.terasology, with some classes elided for brevity:

terasology
├── input
│   └── binds
│       └── inventory
│           ├── DropItemButton.java
│           ├── InventoryButton.java
│           ├── ToolbarNextButton.java [ … ]
│           └── ToolbarSlotButton.java
├── logic
│   └── inventory
│       ├── action
│       │   ├── GiveItemAction.java
│       │   ├── MoveItemAction.java
│       │   ├── RemoveItemAction.java
│       │   └── SwitchItemAction.java
│       ├── block
│       │   ├── BlockInventorySystem.java
│       │   ├── DropBlockInventoryComponent.java
│       │   └── RetainBlockInventoryComponent.java
│       ├── CharacterInventorySystem.java
│       ├── events
│       │   ├── AbstractMoveItemRequest.java
│       │   ├── BeforeItemPutInInventory.java  [ … ]
│       │   ├── MoveItemRequest.java
│       │   └── MoveItemToSlotsRequest.java
│       ├── InventoryAccessComponent.java
│       ├── InventoryAuthoritySystem.java
│       ├── InventoryClientSystem.java
│       ├── InventoryComponent.java [ … ]
│       └── StartingInventorySystem.java
└── rendering
    └── nui
        └── layers
            ├── hud
            │   ├── DropItemRegion.java
            │   └── InventoryHud.java
            └── ingame
                └── inventory
                    ├── BeforeInventoryCellRendered.java
                    ├── ContainerScreen.java
                    ├── GetItemTooltip.java
                    ├── InventoryCell.java [ … ]
                    └── TransferItemCursor.java

What do we want it to look like instead? The root at org.terasology.module.inventory, and then drop inventory out of the middle layers?

org.terasology.logic.inventory.action.GiveItemAction becomes
org.terasology.module.inventory.logic.action.GiveItemAction

these are getting lengthy. Are there any levels we might collapse while we're at it? (I'm looking at rendering.nui.layers suspiciously.)

keturn commented 3 years ago

for modules I'd probably even be okay with dropping the logic and input levels, so

org.terasology.logic.inventory.action.GiveItemAction becomes org.terasology.module.inventory.action.GiveItemAction

if Component and System classes end up in the org.terasology.module.inventory root because of that, that seems okay to me. Most modules don't declare very many of those, right?

skaldarnar commented 3 years ago

I'd prefer to use terminology from the ECS here (also using plural if this is a package containing multiple different incarnations of systems or events):

~~logic~~ systems
~~action~~ events

I'd flatten rendering.nui.layers.{hud,ingame} to just ui, i.e. org.terasology.module.inventory.ui.

Would this be too flat?

org.terasology.module.inventory
├── input
│   ├── DropItemButton.java
│   ├── InventoryButton.java
│   ├── ToolbarNextButton.java [ … ]
│   └── ToolbarSlotButton.java
├── components
│   ├── DropBlockInventoryComponent.java
│   ├── RetainBlockInventoryComponent.java
│   ├── InventoryAccessComponent.java
│   └── InventoryComponent.java [ … ]
├── events
│   ├── GiveItemAction.java
│   ├── MoveItemAction.java
│   ├── RemoveItemAction.java
│   ├── SwitchItemAction.java
│   ├── AbstractMoveItemRequest.java
│   ├── BeforeItemPutInInventory.java  [ … ]
│   ├── MoveItemRequest.java
│   └── MoveItemToSlotsRequest.java
├── systems
│   ├── BlockInventorySystem.java
│   ├── CharacterInventorySystem.java
│   ├── InventoryAuthoritySystem.java
│   ├── InventoryClientSystem.java
│   └── StartingInventorySystem.java
└── ui
    ├── DropItemRegion.java
    ├── InventoryHud.java
    ├── BeforeInventoryCellRendered.java
    ├── ContainerScreen.java
    ├── GetItemTooltip.java
    ├── InventoryCell.java [ … ]
    └── TransferItemCursor.java

Maybe break up events into sub-pacakges:

├── events
│   ├── BeforeItemPutInInventory.java  [ … ]
│   ├── actions
│   │  ├──GiveItemAction.java
│   │  ├──MoveItemAction.java
│   │  ├── RemoveItemAction.java
│   │  └── SwitchItemAction.java
│   └── requests
│      ├── AbstractMoveItemRequest.java
│      ├── MoveItemRequest.java
│      └── MoveItemToSlotsRequest.java

Terasology / Inventory

organize under single `inventory` package #37