Added Buffers (item and power) #455

Closed
Trytoon wants to merge 13 commits from my6.0.11 into dev/1.20.1
Trytoon commented 2023-08-12 11:27:29 +00:00 (Migrated from github.com)

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.
# 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 <!-- Follow this exact pattern for every issue you've fixed to help GitHub automatically link your PR to the relevant issues --> <!-- For drafts, fill this in as you go; if you are leaving draft, make sure these are all done --> # Checklist: - [X] My code follows the style guidelines of this project (.editorconfig, most IDEs will use this for you). - [X] I have performed a self-review of my own code. - [X] I have commented my code in areas it may be challenging to understand. <!-- (Although we prefer code that is readable instead of over-commented) --> - [X] I have made corresponding changes to the documentation. - [X] My changes are ready for review from a contributor. <!-- Thanks to: https://embeddedartistry.com/blog/2017/08/04/a-github-pull-request-template-for-your-projects/ for the building blocks of this template -->
dphaldes (Migrated from github.com) requested changes 2023-08-13 19:55:00 +00:00
dphaldes (Migrated from github.com) left a comment

Initial review :)

Initial review :)
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
dphaldes (Migrated from github.com) commented 2023-08-13 19:45:44 +00:00

Probably should use lang instead of Raw string

Probably should use lang instead of Raw string
dphaldes (Migrated from github.com) commented 2023-08-13 19:47:10 +00:00

You can probably use onInventoryContentsChanged on BE instead of containerTick. Can someone confirm which is more performant ?

You can probably use `onInventoryContentsChanged` on BE instead of containerTick. Can someone confirm which is more performant ?
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
dphaldes (Migrated from github.com) commented 2023-08-13 19:47:24 +00:00

Use lang

Use lang
@ -0,0 +93,4 @@
try {
DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getInstance(Locale.ROOT);
decimalFormat.applyPattern("#,###");
dphaldes (Migrated from github.com) commented 2023-08-13 19:50:07 +00:00

What happens when we use big numbers like 1M? Maybe future proof it since the widget can be used somewhere else

What happens when we use big numbers like 1M? Maybe future proof it since the widget can be used somewhere else
@ -0,0 +1,240 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2023-08-13 19:51:39 +00:00

Put NBT keys in CoreNBTKeys file

Put NBT keys in CoreNBTKeys file
Trytoon (Migrated from github.com) reviewed 2023-08-13 19:57:13 +00:00
@ -0,0 +93,4 @@
try {
DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getInstance(Locale.ROOT);
decimalFormat.applyPattern("#,###");
Trytoon (Migrated from github.com) commented 2023-08-13 19:57:13 +00:00

I tested this function in a normal java program just to see how the decimal format behaves and it works fine with bigger numbers

I tested this function in a normal java program just to see how the decimal format behaves and it works fine with bigger numbers
Trytoon (Migrated from github.com) reviewed 2023-08-13 20:08:59 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
Trytoon (Migrated from github.com) commented 2023-08-13 20:08:58 +00:00

I don't think so, we are not updating the inventory when updating I/O rates

I don't think so, we are not updating the inventory when updating I/O rates
dphaldes (Migrated from github.com) reviewed 2023-08-14 06:43:01 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
dphaldes (Migrated from github.com) commented 2023-08-14 06:43:01 +00:00

my bad. use Editbox setResponder()

my bad. use Editbox setResponder()
Trytoon (Migrated from github.com) reviewed 2023-08-14 09:38:36 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
Trytoon (Migrated from github.com) commented 2023-08-14 09:38:35 +00:00

I've implemented a version with the setResponder().
However now it updates whenevr a new value is in the textbox but I want them to be updated if and only if the textbox is not selected meaning that the player has fisnished setting the value

We might go back to this version and improve it if we cannot do this we the responder

I've implemented a version with the setResponder(). However now it updates whenevr a new value is in the textbox but I want them to be updated if and only if the textbox is not selected meaning that the player has fisnished setting the value We might go back to this version and improve it if we cannot do this we the responder
dphaldes (Migrated from github.com) requested changes 2023-08-14 12:11:06 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
dphaldes (Migrated from github.com) commented 2023-08-14 12:10:59 +00:00

you dont need this anymore. instead create 2 functions where you set input and output values to BE and use these functions as EditBox responders

you dont need this anymore. instead create 2 functions where you set input and output values to BE and use these functions as EditBox responders
Trytoon (Migrated from github.com) reviewed 2023-08-14 12:21:18 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
Trytoon (Migrated from github.com) commented 2023-08-14 12:21:17 +00:00

Yes this is what was done.

But I need this to automatically reset and format (to have comma separator ) the content to 0 or max energy use when the player removes or upgrade capacitor

Yes this is what was done. But I need this to automatically reset and format (to have comma separator ) the content to 0 or max energy use when the player removes or upgrade capacitor
dphaldes (Migrated from github.com) reviewed 2023-08-14 12:32:54 +00:00
@ -0,0 +1,98 @@
package com.enderio.machines.client.gui.screen;
dphaldes (Migrated from github.com) commented 2023-08-14 12:32:53 +00:00

Yeah do all of that in the functions not in containerTick

Yeah do all of that in the functions not in containerTick
dphaldes (Migrated from github.com) requested changes 2024-01-02 16:49:20 +00:00
@ -44,0 +44,4 @@
public static final String BUFFER_MAX_INPUT = "BufferMaxInput";
public static final String BUFFER_MAX_OUTPUT = "BufferMaxOutput";
dphaldes (Migrated from github.com) commented 2024-01-02 16:02:12 +00:00

Use Generic ENERGY_MAX_RECEIVE and ENERGY_MAX_EXTRACT instead.

Use Generic ENERGY_MAX_RECEIVE and ENERGY_MAX_EXTRACT instead.
@ -0,0 +59,4 @@
@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);
dphaldes (Migrated from github.com) commented 2024-01-02 16:47:29 +00:00

Are you sure this mouseClicked result can be ignored?

Are you sure this mouseClicked result can be ignored?
@ -0,0 +93,4 @@
try {
DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getInstance(Locale.ROOT);
decimalFormat.applyPattern("#,###");
dphaldes (Migrated from github.com) commented 2024-01-02 16:43:32 +00:00

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

Look into `EditTextBox#formatter` and `FormattedSequence`. Is that something that should be used instead?
@ -0,0 +110,4 @@
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);
}
}
dphaldes (Migrated from github.com) commented 2024-01-02 16:44:40 +00:00

Not required anymore

Not required anymore
@ -0,0 +54,4 @@
addDataSlot(outputDataSlot);
inputTextDataSlot = new StringNetworkDataSlot(this::getMaxInputText, input -> inputTextValue = input);
outputTextDataSlot = new StringNetworkDataSlot(this::getMaxOutputText, output -> outputTextValue = output);
dphaldes (Migrated from github.com) commented 2024-01-02 16:19:11 +00:00

Why are both the value and string form synced separately ?

Why are both the value and string form synced separately ?
@ -0,0 +100,4 @@
if (level != null && level.isClientSide) {
clientUpdateSlot(outputTextDataSlot, maxOutputText);
} this.outputTextValue = maxOutputText;
}
dphaldes (Migrated from github.com) commented 2024-01-02 16:20:28 +00:00

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

There is no point in syncing the text. It should be completely client sided. The server needs to know only the final Integer.
@ -0,0 +139,4 @@
}
} else {
if (otherHandler.get().canReceive()) {
dirs.add(side);
dphaldes (Migrated from github.com) commented 2024-01-02 16:25:26 +00:00

Will this take care of neighbors being full or empty ?

Will this take care of neighbors being full or empty ?
@ -0,0 +175,4 @@
int received = otherHandler.get().receiveEnergy(Math.min(energyPerSide, getExposedEnergyStorage().getEnergyStored()), false);
// Consume that energy from our buffer.
getExposedEnergyStorage().extractEnergy(received, false);
dphaldes (Migrated from github.com) commented 2024-01-02 16:27:40 +00:00

Shouldn't these be simulated first ?

Shouldn't these be simulated first ?
@ -130,4 +131,4 @@
builder.pop();
builder.push("drain");
dphaldes (Migrated from github.com) commented 2024-01-02 16:07:22 +00:00

Usage/Consumption ?

Usage/Consumption ?
Rover656 commented 2024-05-03 21:00:38 +00:00 (Migrated from github.com)

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)

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 commented 2025-04-25 22:13:33 +00:00 (Migrated from github.com)

Closing due to PR age - thanks for your work and I hope we're able to revisit the buffers in future :)

Closing due to PR age - thanks for your work and I hope we're able to revisit the buffers in future :)

Pull request closed

Sign in to join this conversation.
No reviewers
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Team-EnderIO/EnderIO#455
No description provided.