search

BVengo / simple-shulker-preview

A minecraft fabric mod that displays a configurable icon indicating the contents of a shulker box, as well as a capacity bar.
GNU Lesser General Public License v3.0
9 stars 9 forks source link

[Feature]: Compatibility for bigger Shulkers #33

Closed sternschnaube closed 20 hours ago

sternschnaube commented 5 months ago

Request Description

Carpet AMS Addition has a feature to double the Shulker size. But the overlay isn't compatible with it.

https://github.com/Minecraft-AMS/Carpet-AMS-Addition

Images

image

The progess bar doesn't scale with the bigger size. image

Fabric Version

0.15.11

Additional Context

No response

BVengo commented 5 months ago

Thanks for this. I'll have to look into it and see how they modify the stack size - hopefully it's in an easily compatible way!

sternschnaube commented 5 months ago

Thank you 😊

BVengo commented 4 months ago

I was looking into this for 1.21, but unfortunately I would need to implement a hard an inefficient work around for the mod you mentioned. I've opened an issue in their repo which allows for an easy compatibility fix though, so we'll see how it goes.

I'll leave this issue open for now - hopefully we can get this working! 1.21 will be released without it though, since my release is already quite late.

wendavid552 commented 4 months ago

I was looking into this for 1.21, but unfortunately I would need to implement a hard an inefficient work around for the mod you mentioned. I've opened an issue in their repo which allows for an easy compatibility fix though, so we'll see how it goes.

I'll leave this issue open for now - hopefully we can get this working! 1.21 will be released without it though, since my release is already quite late.

One of a possible but inefficient method could be calculate the capacity through .stream() instead of .iterateNonEmpty() as this would go through all the slots, including the empty ones here (btw I haven't tested it).

BVengo commented 4 months ago

Yep, I tested that but, strangely, it still only returned non-empty. I suspect there are two different shulker inventories - one for the held item ContainerComponent which only knows the exact contents (what we're handling), and one for the block itself which has the maximum count (which their mod alters).

Their solution involved changing the size() function, but that's per-instance rather than for all shulkers. My hacky solution would be to create a temporary instance and call size(), but this would be super inefficient and have to be done every single tick in case the setting is changed between large and small at some stage.

Ideally, they just update the public static INVENTORY_SIZE variable (or whatever it was called) alongside their other changes for compatibility with other mods.

sternschnaube commented 4 months ago

Thank you both for your efforts 😊

wendavid552 commented 4 months ago

Yep, I tested that but, strangely, it still only returned non-empty. I suspect there are two different shulker inventories - one for the held item ContainerComponent which only knows the exact contents (what we're handling), and one for the block itself which has the maximum count (which their mod alters).

Their solution involved changing the size() function, but that's per-instance rather than for all shulkers. My hacky solution would be to create a temporary instance and call size(), but this would be super inefficient and have to be done every single tick in case the setting is changed between large and small at some stage.

Ideally, they just update the public static INVENTORY_SIZE variable (or whatever it was called) alongside their other changes for compatibility with other mods.

XD I am one of the contributor from Carpet-AMS-Addition😊.

The problem is, our mod is a serverside mod, while simple-shulker-preview is a clientside mod, modifying the constant serverside won't help.

BVengo commented 4 months ago

XD I am one of the contributor from Carpet-AMS-Addition😊.

The problem is, our mod is a serverside mod, while simple-shulker-preview is a clientside mod, modifying the constant serverside won't help.

Oh dang, I never considered that. That puts a spanner in the works!

Will have to see if I can somehow get to the size() function from the objects I have available, because I'd really like to avoid my hacky solution.

Alternatively, I could add a custom config for shulker size modification. Not difficult to do, but I'd like to keep it as generic as possible so I don't like that solution either.

wendavid552 commented 4 months ago

XD I am one of the contributor from Carpet-AMS-Addition😊. The problem is, our mod is a serverside mod, while simple-shulker-preview is a clientside mod, modifying the constant serverside won't help.

Oh dang, I never considered that. That puts a spanner in the works!

Will have to see if I can somehow get to the size() function from the objects I have available, because I'd really like to avoid my hacky solution.

Alternatively, I could add a custom config for shulker size modification. Not difficult to do, but I'd like to keep it as generic as possible so I don't like that solution either.

Hi I wrote a quick fix in #35 by .stream(). It works actually.

If you want to test it with Carpet-AMS-Addition, I have sent a test server to you in Discord.

BVengo commented 4 months ago

Thanks for that. Did some more testing and added discussion to the PR. Unfortunately I'm still not seeing .stream() work!

sternschnaube commented 1 month ago

Any update on this @BVengo? 😊

BVengo commented 1 month ago

Unfortunately not, sorry about that. Life has kept me too busy for non-update development recently.

It's top on my list for bug fixes though, when I'm available again!

sternschnaube commented 1 month ago

Thanks for letting me know 😄

BVengo commented 20 hours ago

I've put a lot of time into investigating this and I'm afraid there's really no way to dynamically handle this until Carpet-AMS-Addition adds a method of communicating with client-side mods. See https://github.com/Minecraft-AMS/Carpet-AMS-Addition/issues/130 for their progress on this.

From a client-side perspective, the inventory simply stores a list of items on the player but does not save the size of the container. The size is handled on the server-side, so client-side mods are stuck assuming that it's 27 stacks as per the vanilla size.

As a workaround, I've now implemented two new options under the Compatibility section allowing you to tell the mod what the size of your shulker boxes really are. There are two downsides to this though.

  1. It effects all shulker boxes - because of the server/client-side communications listed above, there is no way to dynamically adapt for shulker boxes of varying sizes
  2. This mod is designed to be a QoL feature that you set and forget, so configs are per-instance rather than per-server. If you swap between servers with and without the larger shulkers, you'll find yourself needing to change these settings regularly.

This is being implemented from 1.21.3 onwards. Unfortunately I'm rather pressed for time recently, so I may not get a chance to backdate these features. Thank you for your patience on this issue though :slightly_smiling_face: