InteractInventoryEvent.Open event with BlockSnapshot filter not firing

I’m trying to listen to the InteractInventoryEvent.Open event and use the BlockSnapshot from its cause. I have this listener:

@Listener
public void onInventoryOpen(InteractInventoryEvent.Open event, @Filter BlockSnapshot block) {
    ...
}

It is not triggering when interacting with a ShulkerBox. I am assuming it is because of the BlockSnapshot filter not finding a BlockSnapshot in the cause.

When I listen to the event without the filter it fires as expected. I then print out the Cause and find that it does contain a BlockSnapshot. Here’s my code:

@Listener
public void onInventoryOpen(InteractInventoryEvent.Open event) {
	logger.info("Cause: " + event.getCause().toString());
}

Here’s the output of the above code showing that the cause contains a BlockSnapshot:

Cause[  
   Context=Context   [  
      "sponge:block_hit"      =SpongeBlockSnapshot      {  
         worldUniqueId=be4d39bb-46f4-4838-ad23-033e48bba9f1,
         position=(-26,
         65,
         -19         ),
         blockState=minecraft:yellow_shulker_box         [  
            facing=up
         ],
         extendedState=minecraft:yellow_shulker_box         [  
            facing=up
         ]
      },
      "sponge:used_item"      =SpongeItemStackSnapshot      {  
         itemType=ItemAir         {  
            Name=null
         },
         quantity=0
      }
   ],
   Stack=   {  
      EntityPlayerMP      [  
         'VapidLinus'         /103,
         l='temp_0',
         x=-26.15,
         y=65.00,
         z=-21.78
      ]
   }
]

I’m probably doing something wrong so I’d like some clarification on why my filtered event is never fired. What should I do instead to retrieve the BlockSnapshot (or TileEntity, because that’s what I actually need).

Thanks in advance! :tada:

EDIT;
Forgot the version info!

SpongeVanilla version 1.12.2-7.0.0-BETA-337
org.spongepowered:spongeapi:7.0.0-SNAPSHOT

which of the dozens of combos of API version, forge/sf or SV, and version of said server

i guess fairly recent 7.0 versions anyways,

The idea is to move away from having things in the cause stack that are not causes, but provide context of them. An interaction with a block is caused by a player performing the action, the context is information pertaining to that, as the shulker box had nothing to do with the chain of events that lead up to the interacting with it :slight_smile:
There are some events in the system ive seen that seem to show a context as a cause but i thinnk its just a string-representation issue.

Look carefully you will see the blocksnapshot is not the object in a cause, but the object within the context, for the EventContextKeys.BlockHit, so you merely can extract the block being acted on from that.

The shulker box never should have really been part of a cause in the first place, it would be within the rest of the event as one of the transactions or target block in the event normally, and its inclusion previously may have just been in error…

Unaware of any filtering events extending to context at this time, but that may not be long in coming once the ground stops shaking on the cause/context reshaping

1 Like

Thank you, I see now that the BlockSnapshot is part of the Context and not the Cause. I didn’t notice that before, that explains why the filter wasn’t working, I think!

Sorry about not providing the version information, I’ll edit it into the main post after I reply to this.

From how I understand it, I can’t use filters for contexts, so I have to do it the dirty way. I changed my code to:

 @Listener
public void onInventoryOpen(InteractInventoryEvent.Open event) {
	// Abort if player didn't interact with a block
	Optional<BlockSnapshot> block = event.getContext().get(EventContextKeys.BLOCK_HIT);
	if (!block.isPresent()) return;

	// Abort if the block wasn't a ShulkerBox
	// (is there a better way to get the TileEntity from a BlockSnapshot?)
	Optional<TileEntity> tileEntity = block.get().getLocation().get().getTileEntity();
	if (tileEntity.map(te -> te.getType() != TileEntityTypes.SHULKER_BOX).orElse(false)) return;

	logger.info("TileEntity info: {}", tileEntity.get().toString());
	logger.info("TileEntity instanceof ShulkerBox: {}", tileEntity.get() instanceof ShulkerBox);

	ShulkerBox shulkerBox = (ShulkerBox) tileEntity.get();

	logger.info("Interacted with: {}", shulkerBox.toString());
}

Now the event fires properly and I get the BlockSnapshot without any problems. I’m having another problem now though which is less related, so maybe I should create a new thread for it? Anywho, the problem is that the casting to a ShulkerBox results in an exception:

java.lang.ClassCastException: net.minecraft.tileentity.TileEntityShulkerBox cannot be cast to org.spongepowered.api.block.tileentity.carrier.ShulkerBox

Here’s the full output of the logger and the error:
https://hastebin.com/arelimabah.rb

I guess it makes sense that net.minecraft.tileentity.TileEntityShulkerBox cannot be cast to org.spongepowered.api.block.tileentity.carrier.ShulkerBox? Advice on how I’m supposed to do it instead would be greatly appreciated :slight_smile:

Okay, unless you have a mod that you can test that you are designing it for, or can think outside the box that I am in (I don’t use mods at all, just plugins on my server forever and always) to suspect some things to try, let me posit this thinking approach.

  1. Yes, step one best coding is to bail out of the event in the majority of cases that wont apply, start cutting down on checks that dont need to be done, etc… so your approach of bailing out for events that dont have a block-hit is the first step of evolving the ‘what can i try now’ code for those of us who aren’t experts that know the best solution before we touch the keyboard, and coming up with a clumsy or klutzy solution that does the job is permissible if you are able to learn a bit more and understand a bit more about the linkages betwen things, start to take bold trial leaps :slight_smile:

Lets consider though what we have…
We have an event that will listen for the opening of an inventory gui (presumedly), do we need to filter for players only opening it I wonder, or will it trigger from other way of opening an inventory… worth investigating, regardless, I’d just drop a @Root Player player in the event since I’d only be interested in players and might as well let it cast what it can.

We have a context that includes a block-hit. My guess would be that if i was interacting with a minecart chest or a villager, if this event fires, those are not blocks and thus would not be a block-hit but an entity-hit or equivalent.
Also, the only blocks that can be inventory containers are Tile Entities ,normal blocks are not dataholders, just tileentities, things that need to store more information in them that isn’t simple as color or orientation, and may involve additional entities (like a jukebox record info). You may interact with a Noteblock tile entity, but you’ll never .Open an inventory for it, thus, EVERY event that has a block-hit in it should be the block. Of course, the context stuff is rather a bit over the place right now and we sometimes see keys that dont belong, so that perfect theory world of “that IS the block” isn’t quite enough.

But, you know the blockhit should be the shulker… or a furnace or anvil… and yeah, just need to get it. But you should also be able to get it from the event as well, cause/context dont throw out all the other useful info, they just help streamline it and give it more dimension… the event may contain an interaction target, and most certainly it will give the inventory itself straight from the event, so likely that exposes some functions that will let you identify which type of inventory it is, Archetype stuff…

I am curious to sit and make a quick test, but I figured I’d respond first to let you start thinking a bit more and try things while I poke at it too … there might be indeed a bug with shulkerboxes, or some other quirk to deal with their casting.

1 Like

I tried poking at it for a while and it is indeed quite odd, but there may be a basic java reason for it I don’t understand, given that the ShulkerBox is an object with no methods or properties of its own, it is the same as the TileEntityContainer it extends.
So i filed a bug report https://github.com/SpongePowered/SpongeCommon/issues/1545

Blocksnapshots shouldn’t be able to be cast to the ShulkerBox (or a Furnace even) , altthough a blocksnapshot is a locatableblock, which is a locatable, as is shulkerbox/furnace … but disconnected in a way that makes sense.

What you would be able to do is get the location from the blocksnapshot, then get the TileEntity from that location, and should-be able to cast to ShulkerBox. But I did some tests and made a demo that I used to file a bug report (though it may come back 'you dont understand how java works, its pretty clear that you cant do that for that one, while all the others you can…)

      @Listener
         public void onInvOpen(InteractInventoryEvent.Open event) {
             event.getContext().get(EventContextKeys.BLOCK_HIT).ifPresent(blocksnapshot -> {
                 blocksnapshot.getLocation().ifPresent(location -> {
                     location.getTileEntity().ifPresent(tileentity -> {
                         broadcast(Text.builder().append(Text.of(TextColors.GRAY, tileentity.getType().getTileEntityType()))
                                 .append(Text.NEW_LINE)
                                 .append(Text.of(TextColors.DARK_AQUA, "Drppr:" + (tileentity instanceof Dropper)))
                                 .append(Text.of(TextColors.DARK_GREEN, "  Hppr:" + (tileentity instanceof Hopper)))
                                 .append(Text.of(TextColors.BLUE, "  Frnc:" + (tileentity instanceof Furnace)))
                                 .append(Text.of(TextColors.YELLOW, "  Bcn:" + (tileentity instanceof Beacon)))
                                 .append(Text.of(TextColors.LIGHT_PURPLE, "  ShlkrBx:" + (tileentity instanceof ShulkerBox)))
                                 .build()
                                 );                
                     });
                 });
             });

         }

         public void broadcast(Text message) {
             Sponge.getServer().getBroadcastChannel().send(message);
         }

It is rather heavy on the isPresent() due to everything being optional in that chain, and good code practise... even though if you hit a block, it has a location (air-hit, with  nonsense location object, does not appear as block-hit); if it fired the inventory open event, and is a block, its a tile entity for sure...  So we 'know' we could skip the optionals and go straight for gets...

Oh wow, someone responded to the bug report I just filed before pasting here, patched it, and pushed it up.
So, I guess the newer server builds will work for it :slight_smile:
1 Like

It was a bug, and its fixed now :slight_smile:

1 Like

Huh, I really didn’t expect it to be a bug! I thought I was just doing something wrong.

I appreciate all your advice and the effort you put into helping me out with this. You’re a hero, thank you pal! :grinning:

Heah, no problem. I was trying to get my last big plugin updated to 1.12.2 to get onto my playtest server this week, and at the last minute I thought “oooh, I forgot to include shulkerbox containers in this plugin, better add those” and just assumed everything would be the same. As far as my plugin is concerned, they are, as the location holds a tile entity and can be offered my data, but since I hadn’t actually TESTED it as of that time with the shulkerboxes, your post was initially going to be just a quick ‘just do this’ but as I try to test any code I share before sharing it… I found the results disturbing.

Not that I needed to actually declare a ShulkerBox object, but it was the thinking that something was SO different about them that it could defy everything I thought I understood about inheritance :slight_smile: So I had to find out if shulkers were some biazzare object that would mess with things, or it was a bug , just to have a better feeling about my own plugin today :slight_smile:
As it turns out, its POSSIBLE such errors may occur still in the future, as the API showing the inheritances and connections is not the same as actually having the mixin code in place that actually links the sponge object to the minecraft object :slight_smile: Its going to be a very rare one, but not impossible, so now we both have confidence to step up and declare shenannigans :slight_smile:

1 Like