Inventory Locks show up twice in Tinkers' Construct GUI
Darkosto opened this issue ยท 10 comments
Describe the bug
When using certain Tinkers' Construct GUIs the locks will appear more than once (they are doubled):
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:
- Open up Tinker's Station or Tinker's Anvil
- 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
The slot positions are not hardcoded, and very much depends on the menu and positions obtained through the gui.
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?
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.
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!)
I am in the process of making contributions to Tinkers' construct targeting 1.18.2, that should fix the issue.
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.
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.
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:
Thanks for all the hard work you've put into this so far!
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?
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.