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

Added Buffers (item and power) #455

Open
wants to merge 13 commits into
base: dev/1.20.1
Choose a base branch
from

Conversation

Trytoon
Copy link
Contributor

@Trytoon Trytoon commented Aug 12, 2023

Description

I've been absent for a long time now but I've been following your amazing bug fixing adventures...

During this time I worked on buffers (Item + Power) which allows the player to have a 3x3 item buffer and a power buffer that allows the player to control I/O energy rates from the block.

It implements an improved version of the energy textbox widget that I showed you on PR #364 so when this gets in the code base, I will close #364.

I'm not sure about everything in my code as usual so I will be taking your suggestions into consideration :) I've commented that in the code so you will understand what I'm talking about...

Closes #150 , #364

Checklist:

  • My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you).
  • I have performed a self-review of my own code.
  • I have commented my code in areas it may be challenging to understand.
  • I have made corresponding changes to the documentation.
  • My changes are ready for review from a contributor.

@Trytoon Trytoon mentioned this pull request Aug 13, 2023
5 tasks
Copy link
Contributor

@dphaldes dphaldes left a comment

Choose a reason for hiding this comment

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

Initial review :)

@Rover656 Rover656 added Type-Enhancement New feature or enhancement to existing feature. MC-1.20.1 Area-Parity 1.12.2 missing features labels Sep 5, 2023
Comment on lines +45 to +47
public static final String BUFFER_MAX_INPUT = "BufferMaxInput";

public static final String BUFFER_MAX_OUTPUT = "BufferMaxOutput";
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Generic ENERGY_MAX_RECEIVE and ENERGY_MAX_EXTRACT instead.


builder.push("powerBuffer");
POWER_BUFFER_CAPACITY = builder.comment("The base energy capacity in uI.").defineInRange("capacity", 100_000, 1, Integer.MAX_VALUE);
POWER_BUFFER_USAGE = builder.comment("The base energy consumption in uI/t.").defineInRange("usage", 1_000, 1, Integer.MAX_VALUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Usage/Consumption ?

Comment on lines +51 to +57
inputDataSlot = new IntegerNetworkDataSlot(this::getMaxInput, input -> maxInput = input);
outputDataSlot = new IntegerNetworkDataSlot(this::getMaxOutput, output -> maxOutput = output);
addDataSlot(inputDataSlot);
addDataSlot(outputDataSlot);

inputTextDataSlot = new StringNetworkDataSlot(this::getMaxInputText, input -> inputTextValue = input);
outputTextDataSlot = new StringNetworkDataSlot(this::getMaxOutputText, output -> outputTextValue = output);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are both the value and string form synced separately ?

Comment on lines +93 to +103
public void setMaxInputText(String maxInputText) {
if (level != null && level.isClientSide) {
clientUpdateSlot(inputTextDataSlot, maxInputText);
} this.inputTextValue = maxInputText;
}

public void setMaxOutputText(String maxOutputText) {
if (level != null && level.isClientSide) {
clientUpdateSlot(outputTextDataSlot, maxOutputText);
} this.outputTextValue = maxOutputText;
}
Copy link
Contributor

@dphaldes dphaldes Jan 2, 2024

Choose a reason for hiding this comment

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

There is no point in syncing the text. It should be completely client sided. The server needs to know only the final Integer.

Comment on lines +137 to +142
if (otherHandler.get().canExtract()) {
dirs.add(side);
}
} else {
if (otherHandler.get().canReceive()) {
dirs.add(side);
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this take care of neighbors being full or empty ?

Comment on lines +176 to +178
int received = otherHandler.get().receiveEnergy(Math.min(energyPerSide, getExposedEnergyStorage().getEnergyStored()), false);
// Consume that energy from our buffer.
getExposedEnergyStorage().extractEnergy(received, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these be simulated first ?

Comment on lines +79 to +96
public String formatEnergy(String value) {
if (value.isEmpty()) {
return "";
}

int energy;
value = value.replace(",", "");

try {
energy = Integer.parseInt(value);
energy = Math.min(energy, maxEnergy.get());
} catch(Exception e) {
energy = 0;
}

try {
DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getInstance(Locale.ROOT);
decimalFormat.applyPattern("#,###");
Copy link
Contributor

@dphaldes dphaldes Jan 2, 2024

Choose a reason for hiding this comment

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

Look into EditTextBox#formatter and FormattedSequence. Is that something that should be used instead?

Comment on lines +109 to +113
public void renderToolTip(GuiGraphics guiGraphics, int mouseX, int mouseY) {
if (mouseX >= getX() && mouseX <= getX() + width && mouseY >= getY() && mouseY <= getY() + height && !isFocused() && maxEnergy.get() != 0) {
guiGraphics.renderTooltip(displayOn.getMinecraft().font, Component.literal(getValue() +"/" + maxEnergy.get() +" µI/t"), mouseX, mouseY);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not required anymore

@Override
public boolean mouseClicked(double mouseX, double mouseY, int button) {
if (mouseX >= getX() && mouseX <= getX() + width && mouseY >= getY() && mouseY <= getY() + height) {
super.mouseClicked(mouseX, mouseY, button);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this mouseClicked result can be ignored?

@Rover656
Copy link
Member

Rover656 commented May 3, 2024

Will not be merged into dev/1.20.1 - needs rebasing on 20.4+ to for this :) (I know you're likely busy - I'm writing this for tracking purposes)

@Rover656 Rover656 added this to the 1.12.2 Parity Features milestone Jun 11, 2024
@Rover656 Rover656 added the Status-Stale This may not be updated by the OP any further. label Jun 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Parity 1.12.2 missing features MC-1.20.1 Status-Stale This may not be updated by the OP any further. Type-Enhancement New feature or enhancement to existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Power Buffers
4 participants