Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ender fluid cover #2505

Open
wants to merge 23 commits into
base: 1.20.1
Choose a base branch
from
Open

ender fluid cover #2505

wants to merge 23 commits into from

Conversation

Arborsm
Copy link
Contributor

@Arborsm Arborsm commented Dec 6, 2024

What

This PR adds ender cover system.


Implementation Details

The implementation includes new classes for virtual cover.


Outcome

Players can now create customized virtual tanks for fluid management, enhancing the gameplay experience.

# Conflicts:
#	src/main/java/com/gregtechceu/gtceu/common/data/GTItems.java
@Arborsm Arborsm requested a review from a team as a code owner December 6, 2024 15:02
@screret screret changed the title ender fluid cover imp ender fluid cover Dec 6, 2024
@screret screret added the type: feature New feature or request label Dec 6, 2024
@screret
Copy link
Member

screret commented Dec 12, 2024

conflicts

@teh-banana
Copy link
Contributor

Doing some testing of this, and I have some questions / comments. I have bolded the more important / buggy parts of my comment.

Reference Screenshot:
image

I have opened the UI to edit from inside a machine, and the X to close the cover is overlaid with the list of "ender tanks".

Also I have no clue what the large text box does. I tried putting in text and clicking the check mark but it did not save what I thought would be a description? Oh apparently it is a description, just didn't save the first time, and I was unable to find anything in the logs about it. There seems to be some buggy behaviour around this saving information, it seems to get cleared when you open the cover / load the tank colour on another block and the internal tank is empty.

I am curious as to why the hex for the colour has an alpha channel in it too, my only thought is that it gives more options for similar colours, but things using minecraft colours typically don't have an alpha (see other mod's ender tanks which just implement the 16 minecraft colours).

Some more tooltips on the different elements of the UI would help with figuring out what stuff does.

image

I feel like the tooltip or the image on the lock is backwards. To me the icon indicates that the cover is currently in an "unlocked" state aka public, but then the tooltip seems to indicate that clicking on the button will switch the tank to public. Maybe it should say something along the lines of: (also include a description for public when it is public, and the existing private description when it is private)

Current mode: Public/Private
Click to switch mode

I have NOT tested this on a server yet, waiting for another PR to be merged and pulled into this PR.

@Arborsm
Copy link
Contributor Author

Arborsm commented Dec 22, 2024

alpha and tooltip just use ceu things

@Arborsm
Copy link
Contributor Author

Arborsm commented Dec 22, 2024

wait the tooltip is reverse ..

@Arborsm
Copy link
Contributor Author

Arborsm commented Dec 22, 2024

There seems to be some buggy behaviour around this saving information, it seems to get cleared when you open the cover / load the tank colour on another block and the internal tank is empty.

because it's design to auto-delete, and if other ender cover has this channel it will be auto recreated

@screret screret added the Do Not Merge DO NOT MERGE THIS PR YET! label Dec 23, 2024
@screret
Copy link
Member

screret commented Dec 23, 2024

Remove Do Not Merge label once this has been thoroughly tested for edge cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do Not Merge DO NOT MERGE THIS PR YET! type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants