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

stackableShulkersInPlayerInventories has many unintuitive behaviours #83

Open
kikugie opened this issue Dec 13, 2024 · 1 comment
Open

Comments

@kikugie
Copy link

kikugie commented Dec 13, 2024

From testing on Wavetech this rule seems to create issues since updating to 1.21.1:

  • Despite mentioning in the description that it's not the same as stackableShulkerBoxes in Carpet, it behaves in the same way - since 1.20.5 shulker boxes can be stacked in player inventories with the usual carpet rule and both allow stacking on the ground.
  • The rule allows players in creative mode to stack shulker boxes with items, which is an undocumented behaviour. More than that, since this is done by injecting a max stack size component, shulker boxes obtained in survival prior to enabling the rule are unaffected, allowing creation of "corrupted shulkers", which behave differently. This behaviour also creates desyncs, since client sees the tag and attempts to stack the boxes, but then server doesn't allow it.
  • The rule prevents shulker boxes from being stacked by hoppers and reintroduces the comparator signal strength overloading, without documenting it as well. Subjectively, this behaviour is covered by Stackable Shulkers Fix, and installing it makes it an informed choice of the player.

Although in my opinion, stackableShulkersInPlayerInventories is obsolete since it's functionality is provided by Carpet since 1.20.5 and there's a dedicated mod that allows users to customize, which behaviours they want to use. However if you intend on keeping the rule in the mod, I suggest at least renaming it and adding a more comprehensive description to avoid confusion and causing issues.

@senseiwells
Copy link
Collaborator

Your first and third point contradict each other, it in fact doesn't behave the same as carpet's stackableShulkerBoxes, as you state the rule prevents shulker boxes from being stacked by hoppers which carpet's does not. I do agree that the description is now outdated, and should probably say something along the lines.

This will always allow you to stack shulkers manually in your inventory and on the ground, but cannot be stacked by hoppers.

I originally ported this from carpet 1.12, hence the name, I don't think this really needs changing.

Being able to stack shulker boxes with items in creative move is a bug, unless you have the other respective rule enabled, and I was unaware of this behaviour.

The desynced shulker boxes issue is also a bug which will be fixed, but can be worked around by relogging, I don't really see uses constantly enabling and disabling the rule and running into this issue.

The comparator signal strength overloading is intentional, with a accompanying rule to disable it stackableShulkerComparatorOverloadFix with the description "Fixes stacked shulkers overloading comparators".

I have no intentions of removing this rule; it has been a feature of this mod for a very long time, and as stated above does provide different behaviour to the carpet implementation. The fact you implemented it in your own mod is great, it gives users more options but that doesn't necessitate stackableShulkersInPlayerInventories obsolescence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants