Team-EnderIO/EnderIO
1
0
Fork 0
Code Issues 114 Pull requests 13 Projects Releases 79 Packages Wiki Activity Actions

Add Impulse Hopper #73

Merged
dphaldes merged 9 commits from dev/impulse into dev/1.19.x 2022-08-13 18:02:55 +00:00
Conversation 19 Commits 9 Files changed 30 +507 -129
dphaldes commented 2022-08-03 09:47:20 +00:00 (Migrated from github.com)
Copy link

Co-authored-by: ferriarnus ferriarnus@users.noreply.github.com

Description

The Impulse Hopper. Updates #23 to 1.19.x

Todo

  • Model
  • Dragging behavior
  • Power and itemtransfer
  • Progress widget

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
Co-authored-by: ferriarnus <ferriarnus@users.noreply.github.com> # Description The Impulse Hopper. Updates #23 to 1.19.x # Todo <!-- Remove this section if you're submitting an already-complete PR --> - [x] Model - [x] Dragging behavior - [x] Power and itemtransfer - [x] Progress widget # 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, particularly in hard-to-understand areas - [x] 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) reviewed 2022-08-03 12:30:23 +00:00
src/api/java/com/enderio/api/capacitor/Scalers.java
@ -7,6 +7,7 @@ public enum Scalers implements IScaler {
FIXED((v, l) -> v),
justliliandev (Migrated from github.com) commented 2022-08-03 12:27:18 +00:00
Copy link

Is that really the right formula?
DEV_ENERGY_TRANSFER has 120 as base value, which would be for
level 1: 14400 FE
level 2: 57600 FE
level 3: 129600 FE
I guess the correct formula would be v * Math.pow(l,2)
but the normal pow has the same "problem", we might need to revisit that

Is that really the right formula? DEV_ENERGY_TRANSFER has 120 as base value, which would be for level 1: 14400 FE level 2: 57600 FE level 3: 129600 FE I guess the correct formula would be `v * Math.pow(l,2)` but the normal pow has the same "problem", we might need to revisit that
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-08-03 12:11:50 +00:00
Copy link

reverse checking order as shouldActTick is the likely faster computation

reverse checking order as shouldActTick is the likely faster computation
justliliandev (Migrated from github.com) commented 2022-08-03 12:12:44 +00:00
Copy link

rename to follow java standards

    public boolean shouldActTick() {// TODO General tick method for power consuming devices?
rename to follow java standards ```suggestion public boolean shouldActTick() {// TODO General tick method for power consuming devices? ```
justliliandev (Migrated from github.com) commented 2022-08-03 12:14:43 +00:00
Copy link

use one type of spacing

use one type of spacing
dphaldes (Migrated from github.com) reviewed 2022-08-03 17:24:04 +00:00
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2022-08-03 17:24:03 +00:00
Copy link

Fixed in next commit

Fixed in next commit
dphaldes (Migrated from github.com) reviewed 2022-08-03 17:24:09 +00:00
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2022-08-03 17:24:09 +00:00
Copy link

Fixed in next commit

Fixed in next commit
dphaldes (Migrated from github.com) reviewed 2022-08-03 17:24:58 +00:00
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2022-08-03 17:24:58 +00:00
Copy link

Fixed in next commit

Fixed in next commit
dphaldes (Migrated from github.com) reviewed 2022-08-03 17:38:31 +00:00
src/api/java/com/enderio/api/capacitor/Scalers.java
@ -7,6 +7,7 @@ public enum Scalers implements IScaler {
FIXED((v, l) -> v),
dphaldes (Migrated from github.com) commented 2022-08-03 17:38:31 +00:00
Copy link

@ferriarnus Can you check if this is intended?

@ferriarnus Can you check if this is intended?
ferriarnus (Migrated from github.com) reviewed 2022-08-04 07:27:19 +00:00
src/api/java/com/enderio/api/capacitor/Scalers.java
@ -7,6 +7,7 @@ public enum Scalers implements IScaler {
FIXED((v, l) -> v),
ferriarnus (Migrated from github.com) commented 2022-08-04 07:27:19 +00:00
Copy link

Hmm could be. I added it a while ago based on 1.12, but it seems agnor is right.

Hmm could be. I added it a while ago based on 1.12, but it seems agnor is right.
Rover656 (Migrated from github.com) requested changes 2022-08-04 23:48:34 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

Mostly a handful of questions, looking good though 👍

Mostly a handful of questions, looking good though 👍
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
Rover656 (Migrated from github.com) commented 2022-08-04 23:48:15 +00:00
Copy link

Don't worry about these capacitor todos. I imagine this PR will merge before #72 so I'll handle these here :)
Not anything needing changed, more of a dev note :)

Don't worry about these capacitor todos. I imagine this PR will merge before #72 so I'll handle these here :) Not anything needing changed, more of a dev note :)
src/machines/java/com/enderio/machines/common/io/energy/IMachineEnergyStorage.java
@ -29,3 +31,3 @@
/**
* Consume energy from storage.
* @apiNote This is capped to {@link #getMaxEnergyUse()}, for uncapped use {@link #takeEnergy(int)} instead.
*
Rover656 (Migrated from github.com) commented 2022-08-04 23:46:28 +00:00
Copy link

Maybe instead of this, mutation operations should have a simulate boolean akin to Forge's energy API?

Maybe instead of this, mutation operations should have a simulate boolean akin to Forge's energy API?
src/machines/java/com/enderio/machines/common/menu/GhostMachineSlot.java
@ -20,3 +20,3 @@
throw new RuntimeException("Ghost slot can be externally modified!!");
if (!layout.guiCanInsert(index) || !layout.guiCanExtract(index))
if (!layout.guiCanInsert(index))
throw new RuntimeException("Ghost slot cannot be modified by the player!");
Rover656 (Migrated from github.com) commented 2022-08-04 23:47:07 +00:00
Copy link

Why was this change again @ferriarnus, I remember you explained it but it slips my mind

Why was this change again @ferriarnus, I remember you explained it but it slips my mind
src/machines/java/com/enderio/machines/common/menu/ImpulseHopperMenu.java
@ -0,0 +1,39 @@
package com.enderio.machines.common.menu;
Rover656 (Migrated from github.com) commented 2022-08-04 23:47:30 +00:00
Copy link

What is this commented method left for?

What is this commented method left for?
ferriarnus (Migrated from github.com) reviewed 2022-08-04 23:50:29 +00:00
src/machines/java/com/enderio/machines/common/menu/ImpulseHopperMenu.java
@ -0,0 +1,39 @@
package com.enderio.machines.common.menu;
ferriarnus (Migrated from github.com) commented 2022-08-04 23:50:29 +00:00
Copy link

It's one of mine, but I think it was fixed with your slot impl? Or maybe not cause dragging logic is a pain.

It's one of mine, but I think it was fixed with your slot impl? Or maybe not cause dragging logic is a pain.
dphaldes (Migrated from github.com) reviewed 2022-08-05 05:33:25 +00:00
src/machines/java/com/enderio/machines/common/menu/GhostMachineSlot.java
@ -20,3 +20,3 @@
throw new RuntimeException("Ghost slot can be externally modified!!");
if (!layout.guiCanInsert(index) || !layout.guiCanExtract(index))
if (!layout.guiCanInsert(index))
throw new RuntimeException("Ghost slot cannot be modified by the player!");
dphaldes (Migrated from github.com) commented 2022-08-05 05:33:24 +00:00
Copy link

I don't know what this exactly does but trying to open the container in-game without that line causes the exception(thrown under that line) to arise.

I don't know what this exactly does but trying to open the container in-game without that line causes the exception(thrown under that line) to arise.
dphaldes (Migrated from github.com) reviewed 2022-08-07 14:24:04 +00:00
src/machines/java/com/enderio/machines/common/menu/ImpulseHopperMenu.java
@ -0,0 +1,39 @@
package com.enderio.machines.common.menu;
dphaldes (Migrated from github.com) commented 2022-08-07 14:24:04 +00:00
Copy link

Dragging looks like it is fixed. Removing the commented code

Dragging looks like it is fixed. Removing the commented code
dphaldes (Migrated from github.com) reviewed 2022-08-07 14:24:15 +00:00
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2022-08-07 14:24:14 +00:00
Copy link

fixed

fixed
dphaldes commented 2022-08-07 14:28:22 +00:00 (Migrated from github.com)
Copy link

Impulse Hopper should be feature complete now. Please check it.
Only thing left missing are GhostSlot related:

  1. Grey overlay on top of ghostSlot
  2. GhostSlot stack size resizing using mouse scroll.
Impulse Hopper should be feature complete now. Please check it. Only thing left missing are GhostSlot related: 1) Grey overlay on top of ghostSlot 2) GhostSlot stack size resizing using mouse scroll.
dphaldes (Migrated from github.com) reviewed 2022-08-08 06:32:21 +00:00
src/machines/java/com/enderio/machines/common/io/energy/IMachineEnergyStorage.java
@ -29,3 +31,3 @@
/**
* Consume energy from storage.
* @apiNote This is capped to {@link #getMaxEnergyUse()}, for uncapped use {@link #takeEnergy(int)} instead.
*
dphaldes (Migrated from github.com) commented 2022-08-08 06:32:21 +00:00
Copy link

This can be done as this method isn't used anywhere else. Should I make the change ?

This can be done as this method isn't used anywhere else. Should I make the change ?
justliliandev (Migrated from github.com) requested changes 2022-08-08 09:20:58 +00:00
src/machines/java/com/enderio/machines/client/gui/screen/ImpulseHopperScreen.java
@ -0,0 +1,61 @@
package com.enderio.machines.client.gui.screen;
justliliandev (Migrated from github.com) commented 2022-08-08 08:39:28 +00:00
Copy link

add spaces

        for (int i = 0; i < 6; i ++) {
            if (getMenu().getBlockEntity().ghostSlotHasItem(i)) {
add spaces ```suggestion for (int i = 0; i < 6; i ++) { if (getMenu().getBlockEntity().ghostSlotHasItem(i)) { ```
justliliandev (Migrated from github.com) commented 2022-08-08 08:41:20 +00:00
Copy link

formatting

                if (getMenu().getBlockEntity().canPass(i)) {
                    this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 26 , 200, 9, 18, 9);
                } else {
                    this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 26 , 200, 0, 18, 9);
                }
                if (getMenu().getBlockEntity().canHold(i)) {
                    this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 53, 200, 9, 18, 9);
                } else {
                    this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 53 , 200, 0, 18, 9);
                }
formatting ```suggestion if (getMenu().getBlockEntity().canPass(i)) { this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 26 , 200, 9, 18, 9); } else { this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 26 , 200, 0, 18, 9); } if (getMenu().getBlockEntity().canHold(i)) { this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 53, 200, 9, 18, 9); } else { this.blit(pPoseStack, getGuiLeft() + 43 + (18*i), getGuiTop() + 53 , 200, 0, 18, 9); } ```
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +1,102 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-08-08 08:41:52 +00:00
Copy link
        if (shouldActTick() && shouldPassItems()) {
```suggestion if (shouldActTick() && shouldPassItems()) { ```
justliliandev (Migrated from github.com) commented 2022-08-08 08:47:10 +00:00
Copy link

first check the condition of the second check as that one should be faster.
then checkfor empty as that one is faster. and or that check with ItemStack#tagMatches, as this will check for NBT and CapNBT and not only if the items are identical

first check the condition of the second check as that one should be faster. then checkfor empty as that one is faster. and or that check with ItemStack#tagMatches, as this will check for NBT and CapNBT and not only if the items are identical
justliliandev (Migrated from github.com) commented 2022-08-08 09:16:28 +00:00
Copy link

make methods private that should not be called from outside

make methods private that should not be called from outside
src/machines/java/com/enderio/machines/common/blockentity/ImpulseHopperBlockEntity.java
@ -0,0 +82,4 @@
result = stack.copy();
result.setCount(ghost.getCount());
} else if (stack.is(result.getItem())) {
result.setCount(result.getCount() + ghost.getCount());
justliliandev (Migrated from github.com) commented 2022-08-08 09:17:32 +00:00
Copy link

Is this check not always true, because of the check in canPass? So the last else with a continue should never be able to run

Is this check not always true, because of the check in canPass? So the last else with a continue should never be able to run
src/machines/java/com/enderio/machines/common/init/MachineBlockEntities.java
@ -48,6 +46,8 @@ public class MachineBlockEntities {
public static final BlockEntityEntry<SlicerBlockEntity> SLICE_AND_SPLICE = register("slice_and_splice", SlicerBlockEntity::new,
MachineBlocks.SLICE_AND_SPLICE);
justliliandev (Migrated from github.com) commented 2022-08-08 09:17:54 +00:00
Copy link
    public static final BlockEntityEntry<ImpulseHopperBlockEntity> IMPULSE_HOPPER = register("impulse_hopper", ImpulseHopperBlockEntity::new,
```suggestion public static final BlockEntityEntry<ImpulseHopperBlockEntity> IMPULSE_HOPPER = register("impulse_hopper", ImpulseHopperBlockEntity::new, ```
src/machines/java/com/enderio/machines/common/menu/ImpulseHopperMenu.java
@ -0,0 +1,39 @@
package com.enderio.machines.common.menu;
justliliandev (Migrated from github.com) commented 2022-08-08 09:19:52 +00:00
Copy link

wrong Nullable Annotation (use jetbrains) and import it instead of the fullname

wrong Nullable Annotation (use jetbrains) and import it instead of the fullname
justliliandev (Migrated from github.com) commented 2022-08-08 09:20:27 +00:00
Copy link
            for (int j = 0; j < 2; ++j) {
                for (int k = 0; k < 6; ++k) {
                    this.addSlot(new MachineSlot(blockEntity.getInventory(), k + j * 6, 8 + 36 + k * 18, 9 + j * 54));
                }
            }
            for (int k = 0; k < 6; ++k) {
```suggestion for (int j = 0; j < 2; ++j) { for (int k = 0; k < 6; ++k) { this.addSlot(new MachineSlot(blockEntity.getInventory(), k + j * 6, 8 + 36 + k * 18, 9 + j * 54)); } } for (int k = 0; k < 6; ++k) { ```
Rover656 (Migrated from github.com) reviewed 2022-08-09 17:22:58 +00:00
src/machines/java/com/enderio/machines/common/io/energy/IMachineEnergyStorage.java
@ -29,3 +31,3 @@
/**
* Consume energy from storage.
* @apiNote This is capped to {@link #getMaxEnergyUse()}, for uncapped use {@link #takeEnergy(int)} instead.
*
Rover656 (Migrated from github.com) commented 2022-08-09 17:22:58 +00:00
Copy link

I'd say so, aye please

I'd say so, aye please
dphaldes (Migrated from github.com) reviewed 2022-08-10 06:28:50 +00:00
src/machines/java/com/enderio/machines/common/io/energy/IMachineEnergyStorage.java
@ -29,3 +31,3 @@
/**
* Consume energy from storage.
* @apiNote This is capped to {@link #getMaxEnergyUse()}, for uncapped use {@link #takeEnergy(int)} instead.
*
dphaldes (Migrated from github.com) commented 2022-08-10 06:28:50 +00:00
Copy link

Done!

Done!
Rover656 (Migrated from github.com) approved these changes 2022-08-10 11:33:37 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

I'm happy with this, thanks for your hard work!

I'm happy with this, thanks for your hard work!
justliliandev (Migrated from github.com) reviewed 2022-08-12 15:47:38 +00:00
justliliandev (Migrated from github.com) left a comment
Copy link

This is now good codewise, but some things appeared during testing:

  • Renderbug if the block is behind another block of the same type. you might have missed a rendertype somewhere in one of the models, pls check with the alloy smelter as that one looks as intended
    image
  • Renderbug from inside (not really a problem unless people spectate/glitch into a block) with a quad with invalid uv mapping
    image
    image

If you want to I can search for those errors, lmk

This is now good codewise, but some things appeared during testing: * Renderbug if the block is behind another block of the same type. you might have missed a rendertype somewhere in one of the models, pls check with the alloy smelter as that one looks as intended ![image](https://user-images.githubusercontent.com/36055315/184391295-ae5f33d3-7a81-4848-a08a-0da4989d0f77.png) * Renderbug from inside (not really a problem unless people spectate/glitch into a block) with a quad with invalid uv mapping ![image](https://user-images.githubusercontent.com/36055315/184391913-3d03c58e-eb0f-44fe-8b51-85dd45a22c6e.png) ![image](https://user-images.githubusercontent.com/36055315/184391970-f20342ae-b17a-4af0-9371-0f9a9a6f5ade.png) If you want to I can search for those errors, lmk
justliliandev (Migrated from github.com) approved these changes 2022-08-13 15:50:25 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Rover656
justliliandev
Labels
Clear labels   
Area-Assets

   
Area-Backend

Backend work not usually visible to players.

  
Area-Conduits

Everything involving conduits

  
Area-Datapacks

   
Area-Lang

   
Area-Mod Compat

   
Area-Parity

1.12.2 missing features

  
Area-Rendering

Rendering code, might be shaders specific or general.

  
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

High priority, crash/breaking.

  
P-1-Medium

Medium priority. Potentially deleting items or breaking functionality.

  
P-2-Low

Low priority. Not hurting anyone, just annoying.

  
Status-Awaiting Response

Waiting for OP to respond

  
Status-Behind-Flag

This change is available but is placed behind a feature flag.

  
Status-Blocked

Blocked by a specific issue or protocol.

  
Status-Cannot Reproduce

   
Status-Duplicate

This issue or pull request already exists

  
Status-Help Wanted

We need some help to try and track this thing down..

  
Status-Incomplete Report

   
Status-Invalid

This doesn't seem right

  
Status-Needs LTS Backport

Fixed in latest but requires LTS backport.

  
Status-Needs Updating

Needs updating to a newer Minecraft version.

  
Status-Stale

This may not be updated by the OP any further.

  
Status-To Implement

   
Status-Triage

A Triage Member should take a look at this

  
Status-Wontfix

This will not be worked on

  
Status-Wontmerge

This PR won't be merged, but is left now for reference of ongoing or future works.

  
Type-Backport

   
Type-Bug

There is a problem.

  
Type-Documentation

Improvements or additions to documentation

  
Type-Enhancement

New feature or enhancement to existing feature.

  
Type-Question

Further information is requested

  
Type-RFC

Request for comments.

  
Type-Suggestion
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
Milestone
Clear milestone
No items
No milestone
First Alpha Release (v6.0)
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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#73
No description provided.
Delete branch "dev/impulse"

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?