Skip to content

Conversation

@Wunka
Copy link
Contributor

@Wunka Wunka commented Nov 21, 2025

Closes: #2311

I have renamed the inventory to mainInventory and added hotbar and am now just going error by error to get this to work.
I started this in a draft to get a little bit of input how this can be done good.

While we can make somethings work with just getting the mainInventory and hotbar, the Inventory based instructions like depositToAny or depositOrDrop are things were I would like some input. Currently I have done an optional fallback parameter but maybe there are better ways?
Maybe this system should be first done good in a different PR
(I have not yet implemented the fallback only added the parameter)

If we do the parameter there is the question of how should we best serialize this?
I am currently just exploiting that the fallback should not be the same as the dest Inventory.

There is also the Question of how the priority system should work.
Example if I shift click item A into my inventory from a chest:
We now search the hotbar. If we find A the item from the chest goes also there.
But what if we don't find A or there is not enough space for the items from the chest.
Should the mainInventory be searched first for A or should we fill empty spaces inside the hotbar first?
So either:
search(hotbar), search(mainInv), emptySpaces(hotbar), emptySpaces(mainInv)
or
search(hotbar), emptySpaces(hotbar), search(mainInv), emptySpaces(mainInv)

In my opinion the first option is better but maybe others have different opinions.

@Wunka
Copy link
Contributor Author

Wunka commented Nov 21, 2025

The only thing missing now in my opinion is migrations but I don't really know currently how I would do that.
The rest should be done.
I have tested a few things like crafting, putting something into a chest shfiting from the chest with different scenarios like full hotbar, not full hotbar but item in mainInventory.

I in some lines I don't like the how the code looks like example: for([2]?Inventory{self.dest, self.fallback}) |_destInv| { so if someone knows a better way / more beautiful way please tell me (without just coping the entire code based on the inventory)

@Wunka Wunka marked this pull request as ready for review November 21, 2025 20:17
@OneAvargeCoder193 OneAvargeCoder193 moved this to WIP/not ready for review in PRs to review Dec 1, 2025
@Argmaster Argmaster moved this from WIP/not ready for review to Low Priority in PRs to review Jan 2, 2026
@Argmaster Argmaster self-requested a review January 2, 2026 16:16
Copy link
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

I think this PR is too big in its current form. I'd suggest to split off independent things and adapt them to be less hardcoded for just the hotbar.

In the future we probably want to allow crafting from chest ingredients (like in Terraria) and shift clicking/depositing to more than one other inventory (→ accessory slots, multiple open chests, "deposit to nearby chests" like in terraria).

There is also the Question of how the priority system should work.

It should first got into the inventory that already has it, if all inventories have it, then it should start filling up the first one, even if the item exists partially in the others.
so
if(search(A)) emptySpaces(A)
if(search(B)) emptySpaces(B)
emptySpaces(A)
emptySpaces(B)

Currently I have done an optional fallback parameter but maybe there are better ways?

Ideally it should be a slice of arbitrary length.

@IntegratedQuantum IntegratedQuantum moved this from Low Priority to WIP/not ready for review in PRs to review Jan 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: WIP/not ready for review

Development

Successfully merging this pull request may close these issues.

Should the hotbar be separate from the inventory?

2 participants