Skip to content

Added Menu Validity Event - #3401

Open
Mrbysco wants to merge 1 commit into
neoforged:26.2.xfrom
Mrbysco:pr/open
Open

Added Menu Validity Event#3401
Mrbysco wants to merge 1 commit into
neoforged:26.2.xfrom
Mrbysco:pr/open

Conversation

@Mrbysco

@Mrbysco Mrbysco commented Aug 8, 2026

Copy link
Copy Markdown

This PR introduces a PlayerInMenuRangeEvent which can be used to override the distance check that closes a menu when the player is too far away.

This can be used by mods to provide a way to remote access menus.

This PR is an updated version of #1323 but updated to 26.2.x

An example of a usecase is an item that can be bound to a block to open the menu from anywhere (as long as the chunk is loaded):

Screen.Recording.2026-08-08.172219.mp4

@neoforged-automation neoforged-automation Bot added the 26.2 Targeted at Minecraft 26.2 label Aug 8, 2026
@CLAassistant

CLAassistant commented Aug 8, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@neoforged-pr-publishing

Copy link
Copy Markdown
  • Publish PR to GitHub Packages

@sciwhiz12 sciwhiz12 added the enhancement New (or improvement to existing) feature or request label Aug 8, 2026
@ChampionAsh5357

ChampionAsh5357 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

So, this event runs at least once per tick, sometimes multiple depending on network packets. I'm not sure whether this should be sent every tick compared to registering the predicates and looping through them there.

The second issue I see is that this is only maintainable for those who use anything already vanilla implemented. Choosing not to use an existing container implementation or a different menu setup means that the modded container won't be accessed.

In those cases, I'd rather the patch be to the place where AbstractContainerMenu#stillValid is called. You loose the initial distance buffer, but as you were suggesting, you shouldn't need that information to achieve your goals.

I did read through the arguments provided by @HenryLoenwind on the original that go against these thoughts, but I disagree with their opinions. I don't know why you would have to patch out the distance checks instead of making it an override system or one of priority.

@Mrbysco

Mrbysco commented Aug 18, 2026

Copy link
Copy Markdown
Author

Thanks for your response, sorry for not getting back sooner.
Will make an alternative to this PR in the upcoming week to address your concerns.

@HenryLoenwind

HenryLoenwind commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

I did read through the arguments provided by @HenryLoenwind on the original that go against these thoughts, but I disagree with their opinions.

Disclaimer: I haven't looked at the code since I wrote the original PR, so I may be outdated, and may also misunderstand the point you're not agreeing with.

As far as I understand, this is about the decision on whether to fire the event before or upstream of the menu's isValid() method. The issue with calling it before is that if the event overrides the distance check, this also disables any additional checks a modded menu may have. So if a menu auto-closes when the player no longer holds a stick in their hand, it would stay open when another mod overrides the distance check. That's a very bad failure mode, in my opinion.

There are 2 ways around this:

  1. Do the check where the distance check is done originally. Any menu relying on the vanilla distance check logic would cleanly and automatically benefit from this. Menus that use a different distance than vanilla would need to manually fire the event to benefit from it but still work as designed.
  2. Move the check to the caller and do the check, event, and then call isValid() if the distance check succeeds. However, this would kill any menu that has a more lenient distance check than the vanilla one, as this would force the vanilla check onto everything. (Breaking change, btw.) Naturally, menus could subscribe to the event to override the value for themselves, but that is cumbersome, and event-stacking always has the potential for bad mod interactions. One of them is disagreement between the menu's event handler and the "remote access" mod's event handler, where it would depend on execution order who would win (effectively, it would be random).

Adding the check and event to the caller and not removing the vanilla distance check would be the worst, as this would rob menus of using stillValid() for anything, as it could not be called if the distance check succeeds because of the event but the vanilla check wouldn't pass.

There is a third possibility, but it would be an even bigger breaking change: Remove the vanilla distance check from the stillValid() chain and create a stillInRange() method for it in all superclasses that has the vanilla logic and the event, then change the caller of stillValid() to call stillValid&& stillInRange(). On paper, this is the cleanest solution, but it also diverges the most from how vanilla works. (A side benefit would be that stillInRange() could take the allowed range as a parameter, making it easier for menus to change their allowed distance (instead of copying the super code with the calculation and event call.)

As a change on vanilla's side, the third option would rule supreme. But in NeoForge? Too divergent from vanilla for my taste. That leaves 1 and 2, and the downsides of 2 are quite massive in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

26.2 Targeted at Minecraft 26.2 enhancement New (or improvement to existing) feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants