search

Shadows-of-Fire / FastWorkbench

Caching recipes in the crafting table
MIT License
23 stars 17 forks source link

Compat issue with Not Enough Creativity #65

Closed Brittank88 closed 2 years ago

Brittank88 commented 3 years ago

Whether the 2x2 crafting is vanilla or via the NotEnoughCreativity 2x2 area (refer to unascribed/NotEnoughCreativity#4 for the specific part of this issue that pertains to their mod), attempts to place an item into any of those slots will immediately cause a crash.

I'm no expert with FML, but I did see unascribed state "FastWorkbench is blindly assuming that the player's container crafting grid has been replaced with its own. Arguably, this is a bug in FastWorkbench.". Maybe a better solution exists based on this?

MC Version: 1.16.4 Forge Version: 35.1.37 FastWorkbench Version: 1.16.3-4.4.1 Crash report here!

Thanks for your time, and let me know if anything more is required!

Brittank88 commented 3 years ago

Oops, above was a reference from the wrong issue tracker lmao

unascribed commented 3 years ago

Fixed in Not Enough Creativity.

Shadows-of-Fire commented 3 years ago

Next version of FB is going to be heading out with a coremod to manage that replacement, so it should always be present. @unascribed If you have the time, could you test the old version of NEC with the new version of FB and see if the issue persists?

unascribed commented 3 years ago

New and old versions now crash immediately before you even try to craft something due to an attempt to access the slots list in the constructor. I use a boolean flag to throw away the slots created by the superclass before rebuilding the inventory myself, as I'm forced to extend PlayerScreenHandler due to the type of the player's playerScreenHandler field.

I guess I can work around this on my end by clearing the list when my constructor gets control instead of preventing the list from being added to.

Relevant part of stack trace 
java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
    at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) ~[?:?] {re:classloading}
    at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) ~[?:?] {re:classloading}
    at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) ~[?:?] {re:classloading}
    at java.util.Objects.checkIndex(Objects.java:372) ~[?:?] {}
    at java.util.ArrayList.get(ArrayList.java:459) ~[?:?] {}
    at shadows.fastbench.asm.PlayerContainerHooks.updatePlayerContainer(PlayerContainerHooks.java:42) ~[fastbench:4.5.0] {re:classloading}
    at net.minecraft.inventory.container.PlayerContainer.(PlayerContainer.java:85) ~[?:?] {re:mixin,pl:accesstransformer:B,re:classloading,pl:accesstransformer:B,xf:fml:fastbench:fbplayertransfer,xf:fml:fastbench:fbplayermatrix,xf:fml:fastbench:fbplayertransfer,pl:mixin:APP:notenoughcreativity.mixins.json:AccessorPlayerScreenHandler,pl:mixin:A}
    at com.unascribed.notenoughcreativity.CreativePlusScreenHandler.(CreativePlusScreenHandler.java:49) ~[notenoughcreativity:1.2.3] {re:mixin,re:classloading}
    at com.unascribed.notenoughcreativity.NotEnoughCreativity.updateInventory(NotEnoughCreativity.java:54) ~[notenoughcreativity:1.2.3] {re:mixin,re:classloading}
Shadows-of-Fire commented 3 years ago

Hm... Any chance you can keep slot 0 in place? That specific slot is important as its a SlotCraftingResultExt, not the original slot that was there. Not having that slot will cause weird things to happen.

unascribed commented 3 years ago

I do put a CraftingResultSlot at index 0, but that's after the super constructor returns and I allow additions to the slot list again.

Due to poor past planning that I can't fix now, my four crafting input slots are at some ridiculously high index after the rest of my slots.

Shadows-of-Fire commented 3 years ago

I should probably look more into what NEC does in the first place. Just been way to busy myself

unascribed commented 3 years ago

https://github.com/unascribed/NotEnoughCreativity/blob/1.16/src/main/java/com/unascribed/notenoughcreativity/CreativePlusScreenHandler.java

Note: Yarn mappings. NEC+ (I call it NEC+ for short rather than NEC to avoid confusion with Not Enough Crashes; more poor planning) is a Fabric mod that is automatically converted to a Forge mod via my own ForgeryTools. There is no Forge version source code.

Shadows-of-Fire commented 3 years ago

The input slots are irrelevant as far as how FB works, but most FB vanilla systems assume that the output slot is #0

unascribed commented 3 years ago

After 54a538, it works with the new FastWorkbench build. The result slot still doesn't show any output but works when clicked. That commit removes the compatibility code with the old FastWorkbench and switches out the slot insertion prevention with a clear after the super constructor returns that retains the zeroth slot.

Shadows-of-Fire commented 3 years ago

I'll have to take a look at it in dev and see what's going on. Better than crashing, I guess.

Brittank88 commented 3 years ago

Hi guys, just wanted to double-check if this is fixed now, or still in its semi-fixed state?

It's fine if not, I'm just holding off installing the mod until I know for sure.

unascribed commented 3 years ago

As far as I know, this is fixed.

Result items still don't appear but at least it doesn't crash.

Brittank88 commented 3 years ago

Ah, so it's still in the semi-fix state. You're right though, at least it won't crash! It's just a bit too clunky to not be able to see the output for me to include it yet.

Thanks for getting back to me so fast <3