VacuumChest #22
No reviewers
Labels
No labels
Area-Assets
Area-Backend
Area-Conduits
Area-Datapacks
Area-Lang
Area-Mod Compat
Area-Parity
Area-Rendering
Good first issue
MC-1.19.2
MC-1.20.1
MC-1.20.4
MC-1.20.6
MC-1.21
MC-1.21.1
Modtoberfest
P-0-High
P-1-Medium
P-2-Low
Status-Awaiting Response
Status-Behind-Flag
Status-Blocked
Status-Cannot Reproduce
Status-Duplicate
Status-Help Wanted
Status-Incomplete Report
Status-Invalid
Status-Needs LTS Backport
Status-Needs Updating
Status-Stale
Status-To Implement
Status-Triage
Status-Wontfix
Status-Wontmerge
Type-Backport
Type-Bug
Type-Documentation
Type-Enhancement
Type-Question
Type-RFC
Type-Suggestion
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: Team-EnderIO/EnderIO#22
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "vacuumchest"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
Vacuum Chest
Checklist:
@ -0,0 +1,57 @@package com.enderio.machines.client.gui.screen;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;add space
@ -0,0 +1,73 @@package com.enderio.machines.client.gui.widget;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 {Move FluidRenderCode to either some common class or a Helper class, to reduce duplicated code
undo change
@ -0,0 +1,82 @@package com.enderio.machines.common.blockentity;switch order to allow for instant push out
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
Move that code to a common place for combined usage with magnet
remove sout
remove sout
@ -0,0 +1,113 @@package com.enderio.machines.common.blockentity;remove one tab
here too
as only one fluid should be here, maybe think about using an in integer dataslot as the fluid isn't required to be synced
fix spacing
fix spacing
switch order for instant push out
move to common helper
@ -0,0 +71,4 @@return new XPVacuumMenu(this, inventory, containerId);}@Overridefix spacing
@ -0,0 +1,73 @@package com.enderio.machines.client.gui.widget;Probably me messing with different blit metods and forgot to fully remove it
@ -0,0 +1,82 @@package com.enderio.machines.common.blockentity;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?
@ -0,0 +1,113 @@package com.enderio.machines.common.blockentity;Good idea, hadn't considered that.
@ -0,0 +19,4 @@import net.minecraftforge.fluids.FluidStack;import net.minecraftforge.fluids.capability.templates.FluidTank;public class FluidStackStaticWidget extends EIOWidget {I get why you want this, but I really don't see how to do it properly.
@ -0,0 +19,4 @@import net.minecraftforge.fluids.FluidStack;import net.minecraftforge.fluids.capability.templates.FluidTank;public class FluidStackStaticWidget extends EIOWidget {Alrighty.
I can take a look at it, once it's merged
@ -0,0 +1,48 @@package com.enderio.base.common.util;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
Entities are never null in this context
Rename things in comment:
items -> entity
player -> target
@ -0,0 +1,82 @@package com.enderio.machines.common.blockentity;remove the isEmpty check as it will be done each tick, if no itemEntities are Available. Instead do this in BlockEntitiy#onLoad
fix spelling
@ -0,0 +1,113 @@package com.enderio.machines.common.blockentity;fix spelling error: decrease and increase
@ -0,0 +1,29 @@package com.enderio.machines.common.menu;Remove this check or explain why it's empty
@ -0,0 +1,48 @@package com.enderio.base.common.util;Hmm, true. My intention was to ignore entities that might not existed anymore. Would another check "isremoved" or something like that work?
@ -0,0 +1,48 @@package com.enderio.base.common.util;Yes, but maybe cache them with a weak reference and check if it still exists and do an isRemoved check
@ -0,0 +1,48 @@package com.enderio.base.common.util;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.
@ -1,7 +1,11 @@package com.enderio.base.common.item.tool;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 falsereturn false;}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;this generic isn't necessary, also move the todo comment to e-> true
move this to a helper, because it duplicates code with collectItems
clear list instead of just adding onto it
@ -0,0 +1,113 @@package com.enderio.machines.common.blockentity;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)
@ -0,0 +1,82 @@package com.enderio.machines.common.blockentity;It may not be needed, but my ide does warn me about it. It shouldn't matter though?
Some minor comments, a lot is for my 1.19 review though.
@ -0,0 +1,82 @@package com.enderio.machines.common.blockentity;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 configpublic Builder extractableGUISlot(Builder builder, int count) {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;indentation
@ -0,0 +1,122 @@package com.enderio.machines.common.blockentity.base;newlines
@ -0,0 +35,4 @@add2WayDataSlot(new IntegerDataSlot(() -> this.getRange(), this::setRange, SyncMode.GUI));}@OverrideNote for myself: Might need to add a commonTick() to make this easier, or move this code into another method to reduce duplication
What is this?
@ -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;I think wildcards are fine for this purpose honestly.
I did not make this? Might be some git weirdness? It blames you ;p
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)
@ -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;That's my ide, it does that on save. Is it really an issue?
@ -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;Nah not an issue, was more of a comment if it was done intentionally :)
Closing in favour of #80
Pull request closed