Skip to content

Ender Fluid Link Cover#178

Merged
Yefancy merged 31 commits intoGregTechCEu:masterfrom
kumquat-ir:ender-fluid-link
Nov 4, 2021
Merged

Ender Fluid Link Cover#178
Yefancy merged 31 commits intoGregTechCEu:masterfrom
kumquat-ir:ender-fluid-link

Conversation

@kumquat-ir
Copy link
Copy Markdown
Contributor

@kumquat-ir kumquat-ir commented Oct 5, 2021

What:
Adds Ender Fluid Link Covers (ported/re-implemented from TecTech)

How solved:
Added a system for registering and accessing virtual fluid tanks

Outcome:
Implemented:

  • The cover itself
  • VirtualTankRegistry, a way to manage virtual fluid tanks
  • FluidTankSwitchShim, mainly for getting TankWidget to switch which tank it displays without too much hackery
  • A terminal app for viewing contents of all virtual tanks (currently free/no requirements)
  • Color rectangle widget

Not implemented (yet?):

  • Private tanks

Additional info:
2021-10-09_17 41 18
2021-10-10_00 04 15
2021-10-10_00 00 03
2021-10-10_00 01 16

@kumquat-ir kumquat-ir marked this pull request as ready for review October 10, 2021 03:56
i really should have designed around these from the beginning lol
# Conflicts:
#	src/main/java/gregtech/api/metatileentity/MetaTileEntity.java
#	src/main/java/gregtech/api/pipenet/tile/PipeCoverableImplementation.java
#	src/main/java/gregtech/api/terminal/TerminalRegistry.java
@kumquat-ir
Copy link
Copy Markdown
Contributor Author

Some things that should be discussed:
Recipe
Transfer rate
Tank size (definitely should be lower, but by how much?)

Known issues currently:
Does not seem to be able to pull from pipes, but pushing works fine
Reference counting is broken (again) when traveling between dimensions (possibly just loading/unloading chunks)

Copy link
Copy Markdown
Contributor

@ALongStringOfNumbers ALongStringOfNumbers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get hovering text over the lock, to say it switches from private to public?

In addition, can there be an option to assign a name or a note to the various tanks in the manager app, so it can be recorded what they are being used for?

In the case of entering a custom hex into the identifier, can we have the text being written highlighted in red if an incomplete hex has been entered and the checkerboard is showing? This would also be useful to skip drawing the checkerboard background in some cases where it can be seen through the entry, such as with AAAAAAAA

The Cover itself needs to override getCapability so that it can mark itself as having the Controllable capability so that it can be picked up by the machine controller and be controlled via the machine controller. Not sure if you would also want to return the fluid handler capability for the FluidTankSwitchShim.

Do we want the default capacity to be Max_int, or something like 64 buckets?

Comment thread src/main/java/gregtech/common/covers/CoverEnderFluidLink.java Outdated
too much trouble for what it was worth
and enable tooltips on ImageWidget
@kumquat-ir
Copy link
Copy Markdown
Contributor Author

Default size has been changed to 64 buckets, I feel like having it be MAX_INT would make full block tanks significantly worse

To address a couple other things:

The checkerboard background is meant to see how transparent the selected color is, it showing on incomplete colors was a byproduct of those having at most 15/255 alpha, making them almost fully transparent. Since dynamically changing text color in a TextFieldWidget is not possible without a lot of work, I instead added an icon that only shows up when the color is incomplete.

As for the notes on tanks, I have no idea how to even start implementing the data storage for that

Comment thread src/main/java/gregtech/loaders/recipe/MachineRecipeLoader.java Outdated
@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Oct 25, 2021

As for the notes on tanks, I have no idea how to even start implementing the data storage for that

you mean TankWidget?

@kumquat-ir
Copy link
Copy Markdown
Contributor Author

No, the thing alson suggested above - being able to assign a note to a virtual tank in the app to record what they are used for

@ALongStringOfNumbers
Copy link
Copy Markdown
Contributor

ALongStringOfNumbers commented Oct 30, 2021

Would it be possible to have a button that either spits out a coordinate set, or highlights the position of the cover (or all covers on the frequency) in world in the viewing app?

@kumquat-ir
Copy link
Copy Markdown
Contributor Author

Not easily. The virtual tanks are not inherently linked to anything about the world, all the registry knows about is the name of the tank. I tried to make the system as general as possible, so it could be easily used for anything else that requires virtual tanks.

Doing this would also require the positions of every cover to be tracked, which wouldn't be too difficult, just annoying

could definitely be done though, i will see how annoying it actually is

# Conflicts:
#	src/main/java/gregtech/api/render/Textures.java
@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Nov 3, 2021

I tried it briefly, and everything was fine. But I am a little curious, the channels created are always save for ever? Even if there's no fluid in it, there's no cover in the world which use this channel. Even if i accidentally type in the wrong channel. I'm worried about displaying too long lists in Terminal.

@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Nov 3, 2021

Also, I recommend adding a searching Component to the app to filter the channel according to the fluid when there are too many channels. If you don't mind, I can finish this work and push code

@kumquat-ir
Copy link
Copy Markdown
Contributor Author

The virtual tanks are not saved forever, they are only actually saved to world data if they have a non-default capacity (which is not currently possible to have) or contain fluid in them, so exiting and re-entering the world should remove unused ones.
I used to have a reference counter to properly cull unused tanks, but ran into a lot of issues trying to actually count how many references a tank has, notably with detecting when the covered TE unloads. There's also an issue caused by adding a reference in the constructor, due to how pipes add covers. I think I solved that one, but didn't end up commiting it. If you can find a way to make it actually work reliably, feel free to revert 38d31b8.

The tanks are already sorted alphabetically, but I do agree that having a search filter would be helpful.
Do feel free to add commits to this, you probably know how a lot of this stuff works better than I do

@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Nov 3, 2021

image
image
image
image

i have two clients login (KilaBash and Yefancy), and a server side. KilaBash set three covers as above pictures. i found two fatal issues:

  1. both kilabash and yefancy could accessing the middle one, even if it was set as a prevate cover by kilabash. Is that your intention?
  2. both of them can only get one tank info from the client side like this:
    image

No world data is synchronized to the client.

First issue, I don't know what you think.
The second issue is that only the terminal needs to be considered. If all the logic works well on the server side, there is really no need to synchronize world data to the client side.

If you can make sure that without synchronizing world data to the client, everything works fine. My advice is to get the information from the server side in the app and synchronize the information to the app's client to update widgets

@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Nov 3, 2021

I have simply fixed the sync issue, assuming that your world data does not need to be synced.

rename a couple things to be more accurate
# Conflicts:
#	src/main/java/gregtech/api/terminal/TerminalRegistry.java
@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Nov 4, 2021

nice job!

@Yefancy Yefancy merged commit 35f75ed into GregTechCEu:master Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants