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

Storage monitor gas support #458

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zeng-github01
Copy link

@zeng-github01 zeng-github01 commented Jun 13, 2024

Added support for IAEGasStack and GasStack for the Storage Monitor and its conversion type, when Mekanism Energistics is loaded

I personally think this design is terrible. I think we can consider combined the configuredItem and configuredFluid
into a List and operate on each object in a for loop to make the implementation more human-friendly.

@NotMyWing What do you think?

@zeng-github01 zeng-github01 marked this pull request as draft June 13, 2024 17:46
@NotMyWing
Copy link
Member

I think a better approach would actually be arbitrary IAEStack support for Storage Monitors. A circular dependency is way too suboptimal.

@zeng-github01
Copy link
Author

zeng-github01 commented Jun 14, 2024

I think a better approach would actually be arbitrary IAEStack support for Storage Monitors. A circular dependency is way too suboptimal.

My opinion is to add a new API for monitors, extending from IAEStack

All methods have an empty method body by default. Provide a registered implementation class Handler and traverse the Handler list to determine what the item in hand is.

Without looping, it is impossible to determine what type of marked item the player has in their hand

@zeng-github01
Copy link
Author

Handler is hard to implement for me. How about using existing code first? @NotMyWing

@zeng-github01
Copy link
Author

zeng-github01 commented Oct 19, 2024

@NotMyWing I am rewriting this PR on another local branch and plan to write a handler interface similar to Handler<T extend IAEStack>

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.

2 participants