Refactor detector covers #1659
Refactor detector covers #1659
Conversation
… functions are empty
…d compatibility check for inverted NBT key
…ting redstone signal strength
| protected void setInverted(boolean isInverted) { | ||
| this.isInverted = isInverted; | ||
| } |
There was a problem hiding this comment.
Doesn't this also need the cover holder notifications that are present in toggleInvertedWithNotification?
Right now, setInverted is called from the GUI button presses, and the notifications only happen when the cover is screwdriver clicked. I would imagine that the notifications need to happen for all cases. Which if that is true, I don't see a point to the two different invert toggle methods.
There was a problem hiding this comment.
When setInverted is called from GUI, it will get automatically synced as GUI will handle it. That is why only screwdriving has synchronization.
| import net.minecraft.util.EnumHand; | ||
| import net.minecraft.util.text.TextComponentTranslation; | ||
|
|
||
| public abstract class CoverDetectorBase extends CoverBehavior { |
There was a problem hiding this comment.
Right now all the subclasses of this base class implement the ITickable interface. Would it make sense for this base class to implement the interface instead and move the common functionality here? (I think the common functionality would just be checking the offset timer though)
There was a problem hiding this comment.
I am aware of this but I don't feel that providing custom function which would do exactly same thing as update (except for offset timer check) is great idea. As the added values is too small to obscure fact that cover is ticking which can be easily recognized by the update function.
src/main/java/gregtech/common/covers/detector/CoverDetectorBase.java
Outdated
Show resolved
Hide resolved
What
All basic detect covers had duplicated functionality for handling screwdriver, syncing data and handling option to invert value of redstone signal.
Implementation Details
Introduction of base class
CoverDetectorBasewhich all detector covers extends.New util function for calculating redstone signal strength. And move all redstone utils to own file
RedstoneUtil.Outcome
Detector covers has less of redundant code.
Closes: #1542
Additional Information
All functionality was preserved and testing did not find any problems related to upgrade.
There are 2 additions to lang files. I was able to handle only en_us and ja_jp. Rest of translations has to provided by others.
Potential Compatibility Issues
None expected.
As reading of inverted NBT key for
CoverDetectorEnergyAdvancedwas preserved and normalize for future.