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

Comparator and Redstone interactivity #2262

Closed
wants to merge 30 commits into from

Conversation

Charnuz
Copy link
Contributor

@Charnuz Charnuz commented Dec 18, 2022

This covers most of #2079, with the goal of comparators/dispensers giving players QoL improvements. This would allow players to automate most barrel recipes as well.

Fixes #2079

Comment on lines +92 to +95
// Sorry for this.
// This returns a unique analog value for every permutation of items within slots
@Override
public int getAnalogValue()
Copy link
Member

Choose a reason for hiding this comment

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

This just seems unintuitive to me, imo. Yes, in theory, people could use this to detect which element in a tool rack is present, but it makes much more sense to just output a count of the number of items in the tool rack.

Comment on lines +108 to +115
@Override
public void neighborChanged(BlockState state, Level level, BlockPos pos, Block block, BlockPos neighborPos, boolean s)
{
if (!level.isClientSide && level.hasNeighborSignal(pos))
{
toggleSeal(level, pos, state);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Should barrels (which are pretty much definitionally a big pile o wood with no moving parts), actually react themselves to redstone signals? I'm on the fence for this, because I think comparator outputs are good and can be justified by "magic comparator block doing stuff" but idk, can "magic dust block doing stuff" also justify this?

@eerussianguy thoughts?

If it's contentious we could also add a config option for this doing anything (see #905)


Copy link
Member

Choose a reason for hiding this comment

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

Revert pls.

{
if (!level.isClientSide && level.hasNeighborSignal(pos))
{
toggleSeal(level, pos, state);
Copy link
Member

Choose a reason for hiding this comment

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

Moreover, if we do add config options, one per device ("barrel reacts to redstone", "powderkeg reacts to redstone") I think would be good. For some reason I think the comparator output is a fairly no-brainer but the redstone input I could see being controversial.

@Override
public int getAnalogOutputSignal(BlockState state, Level level, BlockPos pos)
{
return state.getValue(TYPE).ordinal();
Copy link
Member

Choose a reason for hiding this comment

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

imo this should output STAGE, not TYPE ? I understand that makes it harder to automate, but it is more inline with what comparators do - measure the full-ness of things.

@@ -365,6 +367,10 @@
enablePlacingItems = builder.apply("enablePlacingItems").comment("If true, players can place items on the ground with V.").define("enablePlacingItems", true);
usePlacedItemWhitelist = builder.apply("usePlacedItemWhitelist").comment("If true, the tag 'tfc:placed_item_whitelist' will be checked to allow items to be in placed items and will exclude everything else.").define("usePlacedItemWhitelist", false);

innerBuilder.pop().push("dispenser");

dispenserEnableLighting = builder.apply("dispenserEnableAutomation").comment("If true, dispensers will be able to light blocks.").define("dispenserEnableAutomation", true);
Copy link
Member

Choose a reason for hiding this comment

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

config value dispenserEnableLighting should match the name dispenserEnableAutomation

@alcatrazEscapee alcatrazEscapee added the Version: 1.18.x Minecraft 1.18.x label Dec 18, 2022
Charnuz and others added 2 commits December 18, 2022 17:33
…cessor.java

Co-authored-by: Alex O'Neill <35673674+alcatrazEscapee@users.noreply.github.com>
Co-authored-by: Alex O'Neill <35673674+alcatrazEscapee@users.noreply.github.com>
@alcatrazEscapee alcatrazEscapee added the needs update This needs to be updated following comments or changes. label Dec 25, 2022
@alcatrazEscapee alcatrazEscapee removed the needs update This needs to be updated following comments or changes. label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Version: 1.18.x Minecraft 1.18.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comparator and Redstone interactivity.
2 participants