Implement Status widget for machines and covers #1303

PrototypeTrousers · 2020-11-08T19:36:06Z

What:
Proposed system to visually help the user solve issues that may arise while using machines and covers, with a widget that may hint as to the root cause of the problem.

How solved:
A 'situation' is set every time the logic passes, or fails.

The widget will then show the player when the GUI is open, where the problem, if any is.

Outcome:
May hint the player to common issues, like why the machine is not working or the cover may stop working due to rare capability issues.

Additional info:

Possible compatibility issue:
on machines the widget uses the space where previously was the low energy one

LAGIdiot

Thanks for your hard work on this PR. But I think you are trying to mix two things together here which in this case does not blend together that well.

I would like you to split this one to two different PRs. One which will handle multi tank formation to which I added one comment. And second handling loss of capabilities. For which I have this:
If machine does not provide compatibility on some side we have to respect that and it should cover even loss of given compatibility. Let's take for example Brewery and Logistic Pipes. As Brewery does not have output slot it does not have ITEM_HANDLER_CAPABILITY on its output side. Which we respect by not allowing placing Conveyor Cover on it and others should too. By clicking it with screwdriver we will allow input which means ITEM_HANDLER_CAPABILITY will be available which will in end allow placing of Conveyor cover on it.

So to handle this correctly we need to do 2 things:

Correctly handle defaultvalue being null in all getCapability for covers which means proper checks before constructing these delegates and throwing argument null exception if someone constructs them with null.
Inform Cover GUI that cover is missing "required" capability which prevents him from working so player can have some clue for why it does not work - this one may be quite tricky

src/main/java/gregtech/api/metatileentity/SimpleMachineMetaTileEntity.java

src/main/java/gregtech/common/metatileentities/storage/MetaTileEntityTank.java

LAGIdiot · 2020-11-20T11:20:02Z

Honestly I was not expecting you to implement it right away as I did not even finished requirements for it. This will be part of public API so we can't rush it out because reworking it later would be pain. Also I want this to be usable on all GUIs that GTCE (and addons) have so it needs to be quite versatile and easy to use. With as little boiler plaiting on use side as possible. So let me get some analysis on this matter. Suggesting are welcome and this may even change to discussion for quite while.

State types we want to report:
Every state should have it's own icon which should be distinct and recognizable on first sight.

Working (everything is going as expected - recipe is being work on, pump is pumping, ...)
Idle (could work if there was something to work on - no recipe selected, nothing to pump, ...)
Warning (situation is happening which may lead to problem or is preventing work - recipe is using more energy then machine is getting, recipe can't be processed because outputs are full, ...)
Problem (something is not working as expected - recipe stopping and progress being reset because of missing energy, steam can't exhaust, missing capability, ...)
Maybe warning could even be split to two soft and hard

Situations we want to report:
This has to be most forgiving so GTCE or addons can add new ones when need arise or remove them without breaking anything.
We should use GTControlledRegistry for this and have publicly available list of know types, so addons could use them and even add it's own.
List to be build when needed.

Regarding use side there should be just one function which will return widget on specified position, and one that will take Situation and feed it to widget, no situation state caching - all this needs to be in widget itself.

PrototypeTrousers · 2020-11-25T00:53:21Z

I struggled a lot with the registry setup so could really use some tips here.
Basically functions the same as before, but the widget gets from the registry the additional info.

LAGIdiot · 2020-11-28T18:03:06Z

First of all good work! You are really putting up good fight with my requests.

Regarding registry setup. I would do it in same way as we have StoneTypes. Two classes one which contains our Situations registrations and one which is Situation. Situation should handle creation and whole registration process if possible in one go. Constructor should get id, name (just short name not full one which you will need for lang files), and state type as enum value (which will be in widget translated to textures). There is no need for holding counter it will be up to registrant not to register with same ID.

Also passing only id to widget seems bit un-intuitive (for synchronization purposes it is required). Regarding used icons and current texts I would like @pyure to say something as he is more UX person them me. There are also some formatting errors but I believe that you will fix them before posting it for verification.

LAGIdiot · 2020-11-28T18:03:35Z

Also please update name and description of this issue

LAGIdiot

I have to say great job on this PR and keeping with my request. There is another bunch of them.

src/main/java/gregtech/api/Situation.java

src/main/java/gregtech/api/Situations.java

src/main/java/gregtech/api/gui/widgets/SituationWidget.java

src/main/java/gregtech/api/metatileentity/MetaTileEntity.java

src/main/java/gregtech/common/covers/CoverConveyor.java

src/main/java/gregtech/api/Situation.java

LAGIdiot · 2020-12-05T10:32:53Z

src/main/java/gregtech/api/gui/widgets/SituationWidget.java

+    protected TextureArea area;
+    private boolean isVisible = true;
+
+    public SituationWidget(int xPosition, int yPosition, int width, int height, IntSupplier getSituationId) {


Also there is still lingering my previous comment regarding use IntSuplier here

i couldn't find any other way to sync the widget properly while its open without using it here.
setting it to int will only display the changes when the GUI is reopened.

For synchronization you will still need that int id but you can get it from Situation it self

LAGIdiot

I am looking that you are doing some good progress on this. I have added some comments you should take look at.

src/main/java/gregtech/api/cover/CoverBehavior.java

src/main/java/gregtech/api/gui/widgets/SituationWidget.java

src/main/java/gregtech/api/situation/Situation.java

ALongStringOfNumbers · 2020-12-21T07:13:04Z

src/main/java/gregtech/api/capability/impl/AbstractRecipeLogic.java

+        if (this.currentRecipe != null && setupAndConsumeRecipeInputs(this.currentRecipe)) {
+            setupRecipe(this.currentRecipe);
+        } else {
+            metaTileEntity.setSituation(OUTPUT_INVENTORY_FULL);


As a note, the trySearchNewRecipe method is overridden in MetaTileEntityMultiFurnace, so it would not benefit from this output full situation being set here. The situation being set should also be added to MetaTileEntityMultiFurnace.

LAGIdiot · 2021-01-17T09:56:58Z

This looks good most of things I mentioned seems to be resolved. We may need to extend coverage of this to cases like one mentioned by @ALongStringOfNumbers.

I would like to see this in next minor version update. But for that we need extensive testing given it's size and integration. Which may be little problematic given my broken dev environment. But I will get to it.

Anyway thank you for you hard work.

PrototypeTrousers · 2021-03-09T02:17:40Z

LAGIdiot · 2021-03-15T20:17:15Z

I was finally able to take a look at this. Let's start with thank you for not abandoning this in my "absence".

I quickly looked through code and it looks solid, for now I will not do review on it as it needs more work. I think that there was some bad merging done as there are issues with lang file. As some PR touching it which are in master are reverted here (for example numeric formatting and TOP integrations).

I see that you created list of machines which are missing this mechanism which is nice. We may also implement this for Battery buffer, Rock breaker and maybe some other on top of your list.

From quick testing I have stumble onto bug with fisher using missing lang entry when water check fails.

we try to push from from the output side every 5 ticks, so some caching of the state is needed to avoid flickering, due to the rest of the logic running every tick

add capability related situation logic to covers

fix pump "tank exported to is full" logic

syncing still is done by rebuild the situation from the registry by its id

cleanup unused variables in the cyclebuttonwidget (hoovering tooltip moved to widget class)

handle null cases

widget updates really slowly due to the ticking logic

avoid iterating the input slots again.

LAGIdiot added subsystem: covers type: bug Something isn't working labels Nov 9, 2020

LAGIdiot requested changes Nov 9, 2020

View reviewed changes

src/main/java/gregtech/api/metatileentity/SimpleMachineMetaTileEntity.java Outdated Show resolved Hide resolved

src/main/java/gregtech/common/metatileentities/storage/MetaTileEntityTank.java Outdated Show resolved Hide resolved

PrototypeTrousers force-pushed the FixPumpCoverNPE branch 2 times, most recently from c6a4fe7 to bf1286d Compare November 15, 2020 01:47

PrototypeTrousers mentioned this pull request Nov 15, 2020

properly initialize metaTileEntityTank fluid inventory #1311

Closed

This comment has been minimized.

Sign in to view

LAGIdiot added status: help needed Extra attention is needed subsystem: gui type: feature New feature or request and removed type: bug Something isn't working labels Nov 20, 2020

PrototypeTrousers changed the title ~~Fix Null pointer exception on covers~~ Implement Status widget for machines and covers Nov 29, 2020

PrototypeTrousers force-pushed the FixPumpCoverNPE branch 3 times, most recently from 8850a5f to 0a54039 Compare November 30, 2020 03:01

LAGIdiot requested changes Dec 5, 2020

View reviewed changes

PrototypeTrousers force-pushed the FixPumpCoverNPE branch from 9719ca1 to af0b2ef Compare December 8, 2020 02:27

LAGIdiot reviewed Dec 13, 2020

View reviewed changes

ALongStringOfNumbers reviewed Dec 21, 2020

View reviewed changes

LAGIdiot added rsr: minor Release size requirements: Minor and removed status: help needed Extra attention is needed labels Jan 24, 2021

PrototypeTrousers force-pushed the FixPumpCoverNPE branch from 01bffe1 to 3ab1317 Compare March 9, 2021 01:48

LAGIdiot added the release note needed label Mar 15, 2021

PrototypeTrousers added 28 commits March 15, 2021 18:31

error texture goes in the registry

a4b5852

Set up basic machine logic

1217f2a

we try to push from from the output side every 5 ticks, so some caching of the state is needed to avoid flickering, due to the rest of the logic running every tick

Split Status Registry and Entries

54129f1

rename resource to match statustype entry

5861f40

rename to match Situation and Situations values

61c9ab4

add machine controller situation logic to covers and machines

3f59f98

add capability related situation logic to covers

fix recipe logic not updating on world reload

110da14

fix pump "tank exported to is full" logic

Move Situation to its own folder and update its references

7d794b3

moved covers situation to coverbehaviors

c9018f2

move hooveringtooltip to widget class

1830fd7

generate localename from situation name

8af62d7

missing import

a9a4262

if to switch

3a03b77

make loop cheaper

9a00da7

move away from int supplier

b73ed95

syncing still is done by rebuild the situation from the registry by its id

simplify switch, move !isWorkingAllowed back

f705c6d

fix declarations and string for cover localized names

a7e3ca9

cleanup unused functions

667cb11

fix switch

3dab8f8

cleanup unused variables in the cyclebuttonwidget (hoovering tooltip moved to widget class)

reverted unintended logic changes and remove unused functions

e146794

fix widget not syncing on cover GUI

57ebefa

handle null cases

dont check if our capaibily is null

d83c93b

add more situations to the recipe logic

1ae038f

implement fisher situation widget

ac5f094

widget updates really slowly due to the ticking logic

reworked idle logic

0ae37a6

avoid iterating the input slots again.

get error type as string

6d2af09

Add to Air collector logic

8be3ae5

initial steam support

8e7c421

PrototypeTrousers force-pushed the FixPumpCoverNPE branch from 3d8e3ef to 8e7c421 Compare March 15, 2021 21:32

LAGIdiot added the early merge adept Adept for merge early in release cycle for longer testing period label Jun 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Status widget for machines and covers #1303

Implement Status widget for machines and covers #1303

PrototypeTrousers commented Nov 8, 2020 •

edited

Loading

LAGIdiot left a comment

This comment has been minimized.

LAGIdiot commented Nov 20, 2020

PrototypeTrousers commented Nov 25, 2020

LAGIdiot commented Nov 28, 2020

LAGIdiot commented Nov 28, 2020

LAGIdiot left a comment

LAGIdiot Dec 5, 2020

PrototypeTrousers Dec 6, 2020

LAGIdiot Dec 13, 2020

LAGIdiot left a comment

ALongStringOfNumbers Dec 21, 2020

LAGIdiot commented Jan 17, 2021

PrototypeTrousers commented Mar 9, 2021 •

edited

Loading

LAGIdiot commented Mar 15, 2021

Implement Status widget for machines and covers #1303

Are you sure you want to change the base?

Implement Status widget for machines and covers #1303

Conversation

PrototypeTrousers commented Nov 8, 2020 • edited Loading

LAGIdiot left a comment

Choose a reason for hiding this comment

This comment has been minimized.

LAGIdiot commented Nov 20, 2020

PrototypeTrousers commented Nov 25, 2020

LAGIdiot commented Nov 28, 2020

LAGIdiot commented Nov 28, 2020

LAGIdiot left a comment

Choose a reason for hiding this comment

LAGIdiot Dec 5, 2020

Choose a reason for hiding this comment

PrototypeTrousers Dec 6, 2020

Choose a reason for hiding this comment

LAGIdiot Dec 13, 2020

Choose a reason for hiding this comment

LAGIdiot left a comment

Choose a reason for hiding this comment

ALongStringOfNumbers Dec 21, 2020

Choose a reason for hiding this comment

LAGIdiot commented Jan 17, 2021

PrototypeTrousers commented Mar 9, 2021 • edited Loading

LAGIdiot commented Mar 15, 2021

PrototypeTrousers commented Nov 8, 2020 •

edited

Loading

PrototypeTrousers commented Mar 9, 2021 •

edited

Loading