Inventory-Free

Inventory-Free

331k Downloads

Inventory Locks show up twice in Tinkers' Construct GUI

Darkosto opened this issue ยท 10 comments

commented

Describe the bug
When using certain Tinkers' Construct GUIs the locks will appear more than once (they are doubled):

image

After speaking with the Tinkers' dev, he suspects the locked slots are hardcoded location in all UIs, whether it has slots or not. If you iterate the player inventory slots instead of drawing at a constant location it should work. The GUIs in question are actually using 3 sets of inventory slots: left, middle, and right

To Reproduce
Steps to reproduce the behavior:

  1. Open up Tinker's Station or Tinker's Anvil
  2. Observe inventory slot locks are now doubled

Expected behavior
To show only 1 set of locked slots

Versions (please complete the following information):

I am currently using the mod in 1.18.2, but this has been an issue since 1.18.1 so you can use that version to test also.

  • Forge version: 40.0.45
  • Mod Version: 3.1.0
commented

The slot positions are not hardcoded, and very much depends on the menu and positions obtained through the gui.

for(Slot slot : screen.getMenu().slots)

The icons are drawn on through this widget component, which is added to every container screen on initialization.
It seems like tinkers screen is made modular with multiple screens combined into one. For some reason, they all get the same container, meaning that you'll find normal slots for all of them if you check screen.getMenu().
For example, when checking this out in debug mode, the overlay was drawn for TinkerStationScreen and TinkerStationButtonsScreen. I'd expect only the former to have a menu with slots, but instead both seem to have the same menu, so both were rendered.

My initial thought is that tinkers' construct should use some dummy container menu for their module screens such as TinkerStationButtonsScreen. However, I can't really say for sure without knowing the reason behind how they designed their screens.
@KnightMiner do you have any insight on this?

commented

I have no idea the reason behind the screens honestly, all our UI code is pretty old and I am not the maintainer for it, it really needs a rewrite (but I hate rendering enough its hard to motivate myself to rewrite it)

That said, don't screens have a method to get slots so you don't have to get them from the container? Otherwise we would be rendering the inventory items in all our different components. I would suggest checking which slot list is used to print the inventory items in the base UI code and using the same list.

commented

Nope, slots are gotten from the container as far as I can see. Rather, slots are handled in functions such as screen.render() and screen.mouseClicked(), and the parent screen for these module screens does not appear to call these functions anywhere.

Instead, it makes calls to functions defined in ModuleScreen. Some of these functions are overridden as is, while others redirect to screen functions such as to screen.renderBg() or screen.renderLabels(), but none to screen.render() or screen.mouseClicked().

In addition, widgets are normally rendered in the screen.render() function, which is why this issue didn't affect for example InfoPanelScreen, even if it has the same issue with its container. The reason that this issue shows up for TinkerStationButtonsScreen is that its superclass SideButtonsScreen overrides screen.renderBg() to draw all widgets in that function.

I'm now very much of the impression that these module screens need to be better integrated with regular screen functionality, in addition to using more representative containers. (if they even need to be container screens at all!)

commented

I am in the process of making contributions to Tinkers' construct targeting 1.18.2, that should fix the issue.

commented

I just want to let you know that this bug occurs in 1.16.5. I don't know if you're planning a fix for 1.16.5, but again, wanted to let you know.

commented

The bug comes from an improper implementation of the gui side bars in tinkers construct. I'm in the process of undoing it for the latest version of tinkers construct (currently at a standstill as I'm waiting for feedback on SlimeKnights/TinkersConstruct#4841). Because of this, I will not attempt to fix it for older minecraft versions.

commented

Hey @kirderf1

Knightminer sent me the tcon jar file with the recent PR that you setup for them. Is that designed to be a complete fix or does it still require some more work to remove the extra locks in the inventory? Currently, the extra locks are still appearing for me:
image

Thanks for all the hard work you've put into this so far!

commented

There are currently two PRs that you might be talking about, SlimeKnights/TinkersConstruct#4988 and SlimeKnights/TinkersConstruct#4990. Only the latter fixes this specific issue. Maybe the jar was made from the first of the two PRs?

commented

Actually nevermind, it's not from the first of those two PRs. It was probably SlimeKnights/TinkersConstruct#4841. While this PR itself is not recent, it was merged recently.

commented

The fix to this issue has now been merged into SlimeKnights/TinkersConstruct, and should be present in its next release.