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

Crafting input interface terminal supports #368

Merged
merged 8 commits into from Aug 5, 2023

Conversation

harrynull
Copy link

@harrynull harrynull commented Jul 31, 2023

1
2
3

  • Make crafting input hatches show up in interface terminal.
  • Custom name with quartz knife
  • Block highlight in crafting status
  • Does not support the wireless versions

Other half in GTNewHorizons/GT5-Unofficial#2200 which needs to be merged after.

Closes GTNewHorizons/GT-New-Horizons-Modpack#14116
Closes GTNewHorizons/GT-New-Horizons-Modpack#14088

Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

Shouldn't make a hard dep on GT5

@miozune
Copy link
Member

miozune commented Aug 1, 2023

Rather than having hardcoded support for specific tile, adding API for addons would be better. We have large molecular assembler for potential support as well.
Example impl: Add interface (IInterfaceTerminalSupport or whatever you name), and make IInterfaceHost and GT tile to implement it. It can have some methods to provide InvTracker and invoke behavior. Add new registry to IRegistryContainer and accept Class<? extends IInterfaceTerminalSupport> to filter by getMachines. Add getTile to MetaTileEntity with comment explaining it's accessed via reflection.

@Dream-Master Dream-Master requested review from a team August 1, 2023 14:56
@harrynull harrynull marked this pull request as draft August 1, 2023 19:17
@harrynull harrynull marked this pull request as ready for review August 1, 2023 22:01
@harrynull
Copy link
Author

@firenoo @miozune

refactored to remove dependency to gt, and instead exposed an api. would appreciate another look

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

Warning: 2 uncommitted changes
#370

@firenoo firenoo dismissed their stale review August 4, 2023 15:07

stale. will look again later

Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

Sorry for the hassle, we had a discussion on the interface terminal right before this PR came in so I wasn't sure how this would affect those plans

tested a bit and looks fine other than the noted change

}
}
}

if (total != this.diList.size() || missing) {
if (total != this.supportedInterfaces.size() || missing) {
System.out.println(
Copy link

Choose a reason for hiding this comment

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

Debug code should be removed

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. Forgot to remove those. Fixed

Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

AE2FC will need a patch to get this to work, but that can be done after this gets merged (assuming no rush)

Thanks to good design, we don't have to worry about this

@Dream-Master Dream-Master merged commit a6915f5 into master Aug 5, 2023
1 check passed
@Dream-Master Dream-Master deleted the crafting-input-interface-terminal branch August 5, 2023 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants