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

WirelessFluidDetectorCover always manipulates blocks redstone output to true #18177

Closed
3 tasks done
Sanduhr32 opened this issue Nov 30, 2024 · 7 comments
Closed
3 tasks done
Labels
Bug: Minor Mod: GT GregTech and its (former) addons Status: Ready for Developer Issue ready for a developer to pick up and implement

Comments

@Sanduhr32
Copy link

Sanduhr32 commented Nov 30, 2024

Your GTNH Discord Username

san32

Your Pack Version

2.6.1

Your Server

private server

Java Version

Java 17

Type of Server

Don't know

Your Expectation

I tried if the wireless fluid detector cover also manipulates the block it's placed on redstone output like the non-wireless version. I expected it to output a redstone signal matching the threshold ruling.

The Reality

the cover manipulates any block it's placed on to output a redstone signal, regardless of threshold.

Your Proposal

wireless covers in addition to the wireless signal transmission also should correctly manipulate the blocks redstone output to be the same as the signal thats transmitted, like their non-wireless variants.

Final Checklist

  • I have searched this issue tracker and there is nothing similar already. Posting on a closed issue saying the bug still exists will prompt us to investigate and reopen it once we confirm your report.
  • I can reproduce this problem consistently by follow the exact steps I described above, or this does not need reproducing, e.g. recipe loophole.
  • I have asked other people and they confirm they also have this problem by follow the exact steps I described above, or this does not need reproducing, e.g. recipe loophole.
@Sanduhr32 Sanduhr32 added Bug: Minor Status: Triage Issue awaiting triage. Remove once this issue is processed labels Nov 30, 2024
@Sanduhr32
Copy link
Author

if desired, i can also try to patch it to not take up the teams time.

@serenibyss serenibyss added Mod: GT GregTech and its (former) addons Status: Ready for Developer Issue ready for a developer to pick up and implement and removed Status: Triage Issue awaiting triage. Remove once this issue is processed labels Dec 1, 2024
@serenibyss
Copy link
Member

@Sanduhr32 That would be appreciated, thanks!

@Sanduhr32
Copy link
Author

i'll try my best to fix the bug then, that the first init of the cover on a face causes a constant redstone signal.
subsequent cover installs (and presumably inits) do not cause a constant signal.

should the cover also intentionally manipulate the faces redstone output to match the wireless signal?

@Sanduhr32
Copy link
Author

Sanduhr32 commented Dec 1, 2024

I believe i could trace it to here: https://github.com/GTNewHorizons/GT5-Unofficial/blob/5bbee72c52ca26597c14266642aa368b0fe964c6/src/main/java/gregtech/api/metatileentity/CoverableTileEntity.java#L506

Whenever a wireless detector cover is installed with manipulatesSidedRedstoneOutputImpl() returning true, causes the block/side to pick redstone output from mSidedRedstone[side.ordinal()] being initially 15.

I believe thats the bug, redstone output being set to 15.

Any other cover emitting redstone seems to update the redstone value by it's logic being run fast enough.
Which i test in this commit for wireless detector covers

When a cover is removed this code is ran
https://github.com/GTNewHorizons/GT5-Unofficial/blob/5bbee72c52ca26597c14266642aa368b0fe964c6/src/main/java/gregtech/api/metatileentity/CoverableTileEntity.java#L419
setting the sides redstone to 0 (a sane default imo).

@Sanduhr32
Copy link
Author

My mentioned commit in my previous comment seems to it for wireless detector covers.

However i am hesitant to PR, as i'ld like to know whether the "simple & stupid" patch shall be chosen or the default redstone output shall be adjusted to a sane 0 default for each side of coverable tiles.

@Sanduhr32
Copy link
Author

Example images use the discord media proxy, as githubs is currently not working for me :/
Example 01:
Example 01
Example 02:
Example 02

@Sanduhr32
Copy link
Author

I am closing this, as the PR got merged. Thanks everyone involved :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Minor Mod: GT GregTech and its (former) addons Status: Ready for Developer Issue ready for a developer to pick up and implement
Projects
None yet
Development

No branches or pull requests

2 participants