Skip to content

Conversation

@MattAKAFred
Copy link

Searches shulker inventory and replaces items if they appear in a shulker box

Probably wouldn't hurt to have a setting to toggle - outside the scope of this PR

Searches shulker inventory and replaces items if they appear in a shulker box

Probably wouldn't hurt to have a setting to toggle - outside the scope of this PR
Copy link
Member

@RoboMWM RoboMWM left a comment

Choose a reason for hiding this comment

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

So uh, I guess 1.16 now makes it so you can open shulker boxes right within your inventory? Basically I'm trying to figure out why these now need to be treated differently than any other block inventory.

int bestMatchSlot = -1;
int bestMatchShulkerSlot = -1;
int bestMatchStackSize = Integer.MAX_VALUE;
boolean bestMatchShulker = false;
Copy link
Member

Choose a reason for hiding this comment

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

indents look... off. Are you using tabs perhaps?

Copy link
Author

Choose a reason for hiding this comment

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

Forgot to change the default in IDE... will fix

if(im.getBlockState() instanceof ShulkerBox) {
ShulkerBox shulker = (ShulkerBox) im.getBlockState();

for(int j = 0; j < 27; j++) {
Copy link
Member

Choose a reason for hiding this comment

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

Might be better, clearer, and more future-proof to use a foreach loop instead.

@RoboMWM
Copy link
Member

RoboMWM commented Jul 3, 2020

So no, I haven't had time to follow 1.16 - so if you wouldn't mind filling me in what changed with shulker boxes? (Yes, am 2 lazy to look it up myself rn)

@MattAKAFred
Copy link
Author

Nothing changed with shulker boxes, I just thought, in the spirit of making inventory management more automatic it would be a nice added feature. It may be worth adding some permissions so people can enable it as desired.

@RoboMWM
Copy link
Member

RoboMWM commented Jul 3, 2020

Ooooooooooooooh I get it now.

You're using shulker boxes in the player's inventory as an extension of their inventory.

In that case I'd strongly suggest, at the minimum, a config option for this that defaults to false - since IMO I'd be confused if my items were being replaced by items I weren't aware we're being pulled from the shulker box (and yes this would definitely be in scope of this PR).

@RoboMWM
Copy link
Member

RoboMWM commented Jul 3, 2020

If anything you may also want to break this out (which would make it easier to add a config option for, too) into its own helper method(s) so you aren't directly modifying the replacement method

@MattAKAFred
Copy link
Author

Good ideas all around - I'll take a crack at adding permission 'n stuff

Searches shulker inventory and replaces items if they appear in a shulker box

Probably wouldn't hurt to have a setting to toggle - outside the scope of this PR
@RoboMWM
Copy link
Member

RoboMWM commented Jul 16, 2020

The easiest is a config option, but you can go the permissions route if you wish. You'd have to document it though, else the admin would never know how to enable access. Or you could go further and also add a command for players to toggle it.

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

Successfully merging this pull request may close these issues.

2 participants