Minecraftforge: DrawBlockHighlightEvent called for entities and air - either a bug or misleading name

[chylex]

The event is called every render tick, no matter whether the player is looking at a block, entity, or air (MovingObjectType.MISS). I don't know whether it's intended or not, but the name suggests it's only for blocks, and there is no javadoc in the event class that would suggest otherwise (in fact, there's no javadoc at all).

There are quite a few mods I looked up with GitHub's search that assume the event is only called for blocks, which can cause weird bugs if the block they're checking for is at 0,0,0 (default for ENTITY) or right below the player's vision (default for MISS).

The issue was replicated in both 1.8 (latest - 11.14.4.1565) and in 1.7.10. Since 1.8.8 is close, it might be worth renaming the event, or adding new ones in case the modder actually wants to detect entities or air.

[mezz]

The method that the event guards (RenderGlobal.drawSelectionBox) is essentially:

if (this.mc.objectMouseOver.typeOfHit == MovingObjectPosition.MovingObjectType.BLOCK) {
    // do stuff
}

I think it's safe to say it's only meant for blocks.

[diesieben07]

I don't see why we wouldn't keep it this way. Someone might use it to draw something other than block highlights.

[Actuarius]

@diesieben07 added labels [RFC]

[williewillus]

i think it's a bug
It's DrawBLOCK highlight event
Not a render tickevent, which is basically what it is right now :P

At least remove it from entities if anything

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time, and will be closed if no further activity occurs. If this issue was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[chylex]

Has this been adressed? If anything, the bug that reports a player is looking at a 0,0,0 block when they're actually looking at an entity should be fixed, and the other behavior documented.

[williewillus]

by fixing this bug does it remove any ablities modders had to render things (via other events)? If so, the event should be generalized to all RayTraceResult types, otherwise it should be fixed to only fire for blocks.

[chylex]

I think you should be able to do the same by using general render event, and grabbing the MovingObjectPosition in that. If it were to fire for all ray traces, it would be called every render tick anyways, just with a few additional fields.

That is how I assume people are doing it right now anyway, 2 years ago when I opened the issue, I checked mods on GitHub and any code I saw using DrawBlockHighlightEvent was assuming it hit a block and not an entity. Besides, most people wouldn't know they could use it for entities unless they specifically ran into this issue or looked into the MC/Forge source in detail, since the behavior wasn't documented.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[chylex]

Yes, still an issue in 1.12.2-14.23.4.2725.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[chylex]

Has this been adressed in 1.13?

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[chylex]

Still named DrawBlockHighlightEvent and still triggers for entities and misses in 26.0.63.

If you look at for example HWYLA that shows tooltips for blocks and entities the player hovers over, even that doesn't use this event to detect what the player is looking at.

Since Minecraft.getInstance().objectMouseOver is available if mods want to deal with entities/misses, it doesn't make sense for the event to trigger for non-blocks.

[DaemonUmbra]

This event fires before the game filters the RayTraceResult to blocks.
I see a place it can go after the result is filtered but the new context would lack some info that the current context has, requiring the event to be tweaked.

I'm not sure a change is needed because the event's name is based on when it fires, not what it fires for and changing it to be in line with what you have described would require breaking changes that result in less information being provided to listeners of the event.

[chylex]

I'm not sure a change is needed because the event's name is based on when it fires, not what it fires

That is not obvious at all, and as I said in the original post, there's a lot of subtly broken code out there that doesn't check the type of highlight.

Regardless of what the name was supposed to mean originally, if you search for how it's actually used and find about half the mods understand it as 'event that triggers when a block is highlighted' - which it isn't - it's a failure to communicate its purpose.

There could've at least been a warning in the event's javadocs, but just adding that now isn't going to fix it for people updating their mods. Renaming the event (or making a new event with a different name, and deprecating this one for removal in 1.15) should get the idea across that the event isn't just for blocks, and force modders who are updating to look at the event again.

[Unnoen]

https://github.com/MinecraftForge/MinecraftForge/pull/6269 and https://github.com/MinecraftForge/MinecraftForge/pull/6362 have addressed this issue.

And with that, I believe this issue can be closed.

It's done, it's finally over.