VacuumChest #22

Closed
ferriarnus wants to merge 56 commits from vacuumchest into dev/1.18.x
ferriarnus commented 2022-03-11 16:19:41 +00:00 (Migrated from github.com)

Description

The Vacuum Chest. I'm gonna make this a draft so people can easily see it. Should I have a look at filters or not, since this can somewhat use them.

Todo

Vacuum Chest

  • Work with hopper? (general storage input/output)
  • Model
  • Filter?
  • Gui layout

Vacuum Chest

  • Work with tank? (general storage input/output)
  • Model
  • xp liquid?
  • Gui layout

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, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
# Description The Vacuum Chest. I'm gonna make this a draft so people can easily see it. Should I have a look at filters or not, since this can somewhat use them. # Todo <!-- Remove this section if you're submitting an already-complete PR --> # Vacuum Chest - [x] Work with hopper? (general storage input/output) - [ ] Model - [ ] Filter? - [ ] Gui layout # Vacuum Chest - [x] Work with tank? (general storage input/output) - [ ] Model - [x] xp liquid? - [ ] Gui layout # 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation <!-- Thanks to: https://embeddedartistry.com/blog/2017/08/04/a-github-pull-request-template-for-your-projects/ for the building blocks of this template -->
justliliandev (Migrated from github.com) requested changes 2022-03-16 16:15:53 +00:00
@ -0,0 +1,57 @@
package com.enderio.machines.client.gui.screen;
justliliandev (Migrated from github.com) commented 2022-03-16 15:48:51 +00:00

Use EIOLang as this key will also be used by conduits

Use EIOLang as this key will also be used by conduits
justliliandev (Migrated from github.com) commented 2022-03-16 15:49:38 +00:00

Use EIOLang as this key will also be used by conduits

Use EIOLang as this key will also be used by conduits
@ -0,0 +1,53 @@
package com.enderio.machines.client.gui.screen;
justliliandev (Migrated from github.com) commented 2022-03-16 15:49:29 +00:00
		font.draw(pPoseStack, this.getMenu().getBlockEntity().getRange() + "", leftPos + imageWidth - 8 - 12 - 20 - 2 - 8, topPos + 50, 0);

add space

```suggestion font.draw(pPoseStack, this.getMenu().getBlockEntity().getRange() + "", leftPos + imageWidth - 8 - 12 - 20 - 2 - 8, topPos + 50, 0); ``` add space
@ -0,0 +1,73 @@
package com.enderio.machines.client.gui.widget;
justliliandev (Migrated from github.com) commented 2022-03-16 15:52:16 +00:00

why is this line commented out

why is this line commented out
justliliandev (Migrated from github.com) commented 2022-03-16 15:55:17 +00:00

why is this line commented out?

why is this line commented out?
@ -0,0 +19,4 @@
import net.minecraftforge.fluids.FluidStack;
import net.minecraftforge.fluids.capability.templates.FluidTank;
public class FluidStackStaticWidget extends EIOWidget {
justliliandev (Migrated from github.com) commented 2022-03-16 15:51:57 +00:00

Move FluidRenderCode to either some common class or a Helper class, to reduce duplicated code

Move FluidRenderCode to either some common class or a Helper class, to reduce duplicated code
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-03-16 16:01:08 +00:00

switch order to allow for instant push out

switch order to allow for instant push out
justliliandev (Migrated from github.com) commented 2022-03-16 16:06:02 +00:00

cache that result for a few ticks (maybe 5(?)) as this is a rather expensive operation. Also don't module just the global ticktime, but rather modulo the position hash or be hash, so that multiple vacuum chests don't search for item entities in the same tick, balancing that load over multiple ticks

cache that result for a few ticks (maybe 5(?)) as this is a rather expensive operation. Also don't module just the global ticktime, but rather modulo the position hash or be hash, so that multiple vacuum chests don't search for item entities in the same tick, balancing that load over multiple ticks
justliliandev (Migrated from github.com) commented 2022-03-16 16:06:55 +00:00

Move that code to a common place for combined usage with magnet

Move that code to a common place for combined usage with magnet
justliliandev (Migrated from github.com) commented 2022-03-16 16:07:36 +00:00

remove sout

remove sout
justliliandev (Migrated from github.com) commented 2022-03-16 16:07:46 +00:00

remove sout

remove sout
@ -0,0 +1,113 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-03-16 16:08:12 +00:00

remove one tab

remove one tab
justliliandev (Migrated from github.com) commented 2022-03-16 16:08:24 +00:00

here too

here too
justliliandev (Migrated from github.com) commented 2022-03-16 16:11:18 +00:00

as only one fluid should be here, maybe think about using an in integer dataslot as the fluid isn't required to be synced

as only one fluid should be here, maybe think about using an in integer dataslot as the fluid isn't required to be synced
justliliandev (Migrated from github.com) commented 2022-03-16 16:11:38 +00:00

fix spacing

fix spacing
justliliandev (Migrated from github.com) commented 2022-03-16 16:11:46 +00:00

fix spacing

fix spacing
justliliandev (Migrated from github.com) commented 2022-03-16 16:12:17 +00:00

switch order for instant push out

switch order for instant push out
justliliandev (Migrated from github.com) commented 2022-03-16 16:13:07 +00:00

move to common helper

move to common helper
@ -0,0 +71,4 @@
return new XPVacuumMenu(this, inventory, containerId);
}
@Override
justliliandev (Migrated from github.com) commented 2022-03-16 16:11:57 +00:00

fix spacing

fix spacing
ferriarnus (Migrated from github.com) reviewed 2022-03-17 13:27:26 +00:00
@ -0,0 +1,73 @@
package com.enderio.machines.client.gui.widget;
ferriarnus (Migrated from github.com) commented 2022-03-17 13:27:24 +00:00

Probably me messing with different blit metods and forgot to fully remove it

Probably me messing with different blit metods and forgot to fully remove it
ferriarnus (Migrated from github.com) reviewed 2022-03-17 13:30:22 +00:00
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
ferriarnus (Migrated from github.com) commented 2022-03-17 13:30:21 +00:00

Where would I put it. The code is also not fully the same, cause it needs to act differently for each of them. I'm unsure how I would properly do it. Maybe split it in 2-3 methods?

Where would I put it. The code is also not fully the same, cause it needs to act differently for each of them. I'm unsure how I would properly do it. Maybe split it in 2-3 methods?
ferriarnus (Migrated from github.com) reviewed 2022-03-17 13:31:10 +00:00
@ -0,0 +1,113 @@
package com.enderio.machines.common.blockentity;
ferriarnus (Migrated from github.com) commented 2022-03-17 13:31:09 +00:00

Good idea, hadn't considered that.

Good idea, hadn't considered that.
ferriarnus (Migrated from github.com) reviewed 2022-03-23 08:52:44 +00:00
@ -0,0 +19,4 @@
import net.minecraftforge.fluids.FluidStack;
import net.minecraftforge.fluids.capability.templates.FluidTank;
public class FluidStackStaticWidget extends EIOWidget {
ferriarnus (Migrated from github.com) commented 2022-03-23 08:52:44 +00:00

I get why you want this, but I really don't see how to do it properly.

I get why you want this, but I really don't see how to do it properly.
justliliandev (Migrated from github.com) reviewed 2022-03-23 11:42:31 +00:00
@ -0,0 +19,4 @@
import net.minecraftforge.fluids.FluidStack;
import net.minecraftforge.fluids.capability.templates.FluidTank;
public class FluidStackStaticWidget extends EIOWidget {
justliliandev (Migrated from github.com) commented 2022-03-23 11:42:31 +00:00

Alrighty.
I can take a look at it, once it's merged

Alrighty. I can take a look at it, once it's merged
justliliandev (Migrated from github.com) requested changes 2022-03-26 00:19:57 +00:00
@ -0,0 +1,48 @@
package com.enderio.base.common.util;
justliliandev (Migrated from github.com) commented 2022-03-25 23:24:31 +00:00

rename, because the name implies, that it just checks something, but doesn't do something.
moveToPos would be a better name imo.
Also the Description of the method describes it's return value, so remove the duplicate in the description

rename, because the name implies, that it just checks something, but doesn't do something. moveToPos would be a better name imo. Also the Description of the method describes it's return value, so remove the duplicate in the description
justliliandev (Migrated from github.com) commented 2022-03-26 00:01:57 +00:00

Entities are never null in this context

Entities are never null in this context
justliliandev (Migrated from github.com) commented 2022-03-26 00:02:40 +00:00

Rename things in comment:
items -> entity
player -> target

Rename things in comment: items -> entity player -> target
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-03-26 00:12:44 +00:00

remove the isEmpty check as it will be done each tick, if no itemEntities are Available. Instead do this in BlockEntitiy#onLoad

remove the isEmpty check as it will be done each tick, if no itemEntities are Available. Instead do this in BlockEntitiy#onLoad
justliliandev (Migrated from github.com) commented 2022-03-26 00:19:42 +00:00

fix spelling

fix spelling
@ -0,0 +1,113 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-03-26 00:15:50 +00:00

fix spelling error: decrease and increase

fix spelling error: decrease and increase
@ -0,0 +1,29 @@
package com.enderio.machines.common.menu;
justliliandev (Migrated from github.com) commented 2022-03-26 00:19:28 +00:00

Remove this check or explain why it's empty

Remove this check or explain why it's empty
ferriarnus (Migrated from github.com) reviewed 2022-03-27 16:41:03 +00:00
@ -0,0 +1,48 @@
package com.enderio.base.common.util;
ferriarnus (Migrated from github.com) commented 2022-03-27 16:41:02 +00:00

Hmm, true. My intention was to ignore entities that might not existed anymore. Would another check "isremoved" or something like that work?

Hmm, true. My intention was to ignore entities that might not existed anymore. Would another check "isremoved" or something like that work?
justliliandev (Migrated from github.com) reviewed 2022-03-27 17:18:26 +00:00
@ -0,0 +1,48 @@
package com.enderio.base.common.util;
justliliandev (Migrated from github.com) commented 2022-03-27 17:18:25 +00:00

Yes, but maybe cache them with a weak reference and check if it still exists and do an isRemoved check

Yes, but maybe cache them with a weak reference and check if it still exists and do an isRemoved check
ferriarnus (Migrated from github.com) reviewed 2022-03-29 12:38:30 +00:00
@ -0,0 +1,48 @@
package com.enderio.base.common.util;
ferriarnus (Migrated from github.com) commented 2022-03-29 12:38:30 +00:00

I get why you say that, but I'm not certain it is worth it in this case. The list is made every 5 ticks, and if I want to make a list of weak reference I need to iterate over the already existing list again. I'm not certain if we gain anything by using a weak reference.

I get why you say that, but I'm not certain it is worth it in this case. The list is made every 5 ticks, and if I want to make a list of weak reference I need to iterate over the already existing list again. I'm not certain if we gain anything by using a weak reference.
justliliandev (Migrated from github.com) reviewed 2022-04-01 20:00:24 +00:00
@ -1,7 +1,11 @@
package com.enderio.base.common.item.tool;
justliliandev (Migrated from github.com) commented 2022-04-01 19:42:53 +00:00

remove nonnull import, as we usually mark nullable things with nullable instead of non null things with nonnnull

remove nonnull import, as we usually mark nullable things with nullable instead of non null things with nonnnull
@ -0,0 +19,4 @@
public static boolean moveToPos(Entity entity, double x, double y, double z, double speed, double speed4, double collisionDistanceSq) {
if (entity.isRemoved()) { //If the entity no longer exists, return false
return false;
}
justliliandev (Migrated from github.com) commented 2022-04-01 19:52:15 +00:00

add this check to it's callers to remove the entity from the caching lists, if possible

add this check to it's callers to remove the entity from the caching lists, if possible
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-04-01 19:50:52 +00:00
                this.itemEntities.add(new WeakReference(ie));

this generic isn't necessary, also move the todo comment to e-> true

```suggestion this.itemEntities.add(new WeakReference(ie)); ``` this generic isn't necessary, also move the todo comment to e-> true
justliliandev (Migrated from github.com) commented 2022-04-01 19:54:35 +00:00

move this to a helper, because it duplicates code with collectItems

move this to a helper, because it duplicates code with collectItems
justliliandev (Migrated from github.com) commented 2022-04-01 19:55:02 +00:00

clear list instead of just adding onto it

clear list instead of just adding onto it
@ -0,0 +1,113 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-04-01 20:00:06 +00:00

everything from VacuumChestBlockEntity
create common and generic helper for this and the itemEntity finding
parameters you probably will use: list, class, level, pos, range, filter (Predicate)

everything from VacuumChestBlockEntity create common and generic helper for this and the itemEntity finding parameters you probably will use: list, class, level, pos, range, filter (Predicate)
ferriarnus (Migrated from github.com) reviewed 2022-04-05 13:59:02 +00:00
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
ferriarnus (Migrated from github.com) commented 2022-04-05 13:59:02 +00:00

It may not be needed, but my ide does warn me about it. It shouldn't matter though?

It may not be needed, but my ide does warn me about it. It shouldn't matter though?
Rover656 (Migrated from github.com) requested changes 2022-06-21 18:26:22 +00:00
Rover656 (Migrated from github.com) left a comment

Some minor comments, a lot is for my 1.19 review though.

Some minor comments, a lot is for my 1.19 review though.
@ -0,0 +1,82 @@
package com.enderio.machines.common.blockentity;
Rover656 (Migrated from github.com) commented 2022-06-21 18:22:15 +00:00

Could you make this use a predicate? The slots support a BiPredicate which gives you the item stack and the slot number, could do (stack, slot) -> slot != 27

Could you make this use a predicate? The slots support a BiPredicate which gives you the item stack and the slot number, could do `(stack, slot) -> slot != 27`
@ -0,0 +73,4 @@
// Slot config
public Builder extractableGUISlot(Builder builder, int count) {
Rover656 (Migrated from github.com) commented 2022-06-21 18:21:36 +00:00

I might move this into the builder, but dont worry about doing that in this PR.

I might move this into the builder, but dont worry about doing that in this PR.
@ -0,0 +1,113 @@
package com.enderio.machines.common.blockentity;
Rover656 (Migrated from github.com) commented 2022-06-21 18:23:04 +00:00

indentation

indentation
@ -0,0 +1,122 @@
package com.enderio.machines.common.blockentity.base;
Rover656 (Migrated from github.com) commented 2022-06-21 18:24:42 +00:00

newlines

newlines
@ -0,0 +35,4 @@
add2WayDataSlot(new IntegerDataSlot(() -> this.getRange(), this::setRange, SyncMode.GUI));
}
@Override
Rover656 (Migrated from github.com) commented 2022-06-21 18:24:35 +00:00

Note for myself: Might need to add a commonTick() to make this easier, or move this code into another method to reduce duplication

Note for myself: Might need to add a commonTick() to make this easier, or move this code into another method to reduce duplication
@ -3,3 +3,2 @@
import com.enderio.machines.EIOMachines;
import com.enderio.machines.client.gui.screen.*;
import com.enderio.machines.common.menu.*;
import com.enderio.machines.client.gui.screen.AlloySmelterScreen;
Rover656 (Migrated from github.com) commented 2022-06-21 18:25:51 +00:00

I think wildcards are fine for this purpose honestly.

I think wildcards are fine for this purpose honestly.
ferriarnus (Migrated from github.com) reviewed 2022-06-21 18:30:11 +00:00
ferriarnus (Migrated from github.com) commented 2022-06-21 18:30:11 +00:00

I did not make this? Might be some git weirdness? It blames you ;p

I did not make this? Might be some git weirdness? It blames you ;p
Rover656 (Migrated from github.com) reviewed 2022-06-21 18:31:28 +00:00
Rover656 (Migrated from github.com) commented 2022-06-21 18:31:27 +00:00

Ahh might be part of the old energy rework, go ahead and see if its used anywhere, if not just delete the file :P (although try merging upstream first to see if it goes away)

Ahh might be part of the old energy rework, go ahead and see if its used anywhere, if not just delete the file :P (although try merging upstream first to see if it goes away)
ferriarnus (Migrated from github.com) reviewed 2022-06-21 19:48:38 +00:00
@ -3,3 +3,2 @@
import com.enderio.machines.EIOMachines;
import com.enderio.machines.client.gui.screen.*;
import com.enderio.machines.common.menu.*;
import com.enderio.machines.client.gui.screen.AlloySmelterScreen;
ferriarnus (Migrated from github.com) commented 2022-06-21 19:48:38 +00:00

That's my ide, it does that on save. Is it really an issue?

That's my ide, it does that on save. Is it really an issue?
Rover656 (Migrated from github.com) reviewed 2022-06-21 19:55:31 +00:00
@ -3,3 +3,2 @@
import com.enderio.machines.EIOMachines;
import com.enderio.machines.client.gui.screen.*;
import com.enderio.machines.common.menu.*;
import com.enderio.machines.client.gui.screen.AlloySmelterScreen;
Rover656 (Migrated from github.com) commented 2022-06-21 19:55:31 +00:00

Nah not an issue, was more of a comment if it was done intentionally :)

Nah not an issue, was more of a comment if it was done intentionally :)
Rover656 commented 2022-08-17 23:47:35 +00:00 (Migrated from github.com)

Closing in favour of #80

Closing in favour of #80

Pull request closed

Sign in to join this conversation.
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#22
No description provided.