Refined Relocation 2

Refined Relocation 2

5M Downloads

Crash When Placing NetherEx Urn

ChloeDawn opened this issue ยท 9 comments

commented

The game crashes to desktop when placing an Urn of Sorrow from NetherEx whilst sneaking, as you don't check if the tile is one of yours before casting to ISortingGridMember.

Relevant line: BlockHighlightHandler.java#L25

Forge version: 1.12-14.21.1.2424
Mod version: 4.1.4
Crash report: paste.ee/6tCb2

commented

It's not an issue of casting on my side, it's the fact that NetherEx returns something that's not an ISortingGridMember in their getCapability implementation despite having been passed a sorting grid member capability. This should be fixed on NetherEx's side.

commented

You never check if the tile has the capability before trying to get it.

commented

Because it shouldn't be necessary. The capability parameter is passed for a reason, and the JavaDocs state that getCapability should return The requested capability. Returns null when {@link #hasCapability(Capability, EnumFacing)} would return false.

https://github.com/MinecraftForge/MinecraftForge/blob/1.12.x/src/main/java/net/minecraftforge/common/capabilities/ICapabilityProvider.java

commented

Y'all are both right. The standard is to check if it has the capability first, in case someone is depending on that check for some reason. I do in NP, iirc.
Except, disallowing capability attachment, as NetherEx does, is also nonstandard. They should have checks and supercalls, as blay suggests.

commented

Doing extra logic in hasCapability would defeat its point of being a lightweight version and I can't find anything about mods having to call hasCapability before calling getCapability, neither in the Forge Documentation nor in the JavaDocs themselves - other than Forge's hopper hooks doing it.

If it's a standard (which in my opinion would just encourage bad code design by making getCapability needlessly more state-dependent), it needs to be properly documented, because it's far from obvious (compare to Java's map implementations, for example).

commented

The capability system's docs are abysmal, yes.

commented

Fixed on NetherEx's side

commented

But not on yours?

commented

I still haven't been given any confirmation that a hasCapability check prior to getCapability is actually the intended way of doing it. If you can get me some actual sources (preferably official) I'll adapt my code, but until then I see no point.