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

Add Crafter #99

Merged
dphaldes merged 16 commits from dev/crafter into dev/1.19.x 2023-01-31 08:42:16 +00:00
Conversation 13 Commits 16 Files changed 25 +547 -7
dphaldes commented 2022-12-05 17:43:13 +00:00 (Migrated from github.com)
Copy link

Description

Add Crafter

Todo

  • GUI
  • actual block functions
  • Redstone
  • Block model
  • IO config
  • Switch to task
  • Testing
  • switch to slotaccess #101

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 Add Crafter # Todo - [x] GUI - [x] actual block functions - [x] Redstone - [x] Block model - [ ] ~~IO config~~ - [ ] Switch to task - [x] Testing - [x] switch to slotaccess #101 # 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) requested changes 2022-12-29 13:42:02 +00:00
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +1,177 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2022-12-29 10:51:20 +00:00
Copy link

don't use a NonNullLIst, rather code this against the queue interface with an ArrayDeque as impl

don't use a NonNullLIst, rather code this against the queue interface with an ArrayDeque as impl
justliliandev (Migrated from github.com) commented 2022-12-29 11:03:46 +00:00
Copy link

extract this to a tryCraftItem method

extract this to a tryCraftItem method
justliliandev (Migrated from github.com) commented 2022-12-29 11:07:25 +00:00
Copy link

make sure that no empty stack is added to the deque instead of removing all empty itemstacks each tick

make sure that no empty stack is added to the deque instead of removing all empty itemstacks each tick
justliliandev (Migrated from github.com) commented 2022-12-29 13:35:10 +00:00
Copy link

do this check by calling ItemStack#isSameItemSameTags

do this check by calling ItemStack#isSameItemSameTags
justliliandev (Migrated from github.com) commented 2022-12-29 13:36:08 +00:00
Copy link

just use else, because the canMergeOutput does this check implicitly
also grow by the size of the outputbuffer itemstack

just use else, because the canMergeOutput does this check implicitly also grow by the size of the outputbuffer itemstack
justliliandev (Migrated from github.com) commented 2022-12-29 13:37:01 +00:00
Copy link

nope, not necessary

nope, not necessary
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +88,4 @@
public void serverTick() {
tryCraft();
super.serverTick();
processOutputBuffer();
justliliandev (Migrated from github.com) commented 2022-12-29 11:01:48 +00:00
Copy link

move processOutputBuffer call to after the super.serverTick to push something into the outputslot directly after the machine has pushed it's content

move processOutputBuffer call to after the super.serverTick to push something into the outputslot directly after the machine has pushed it's content
src/machines/java/com/enderio/machines/common/io/item/MachineInventoryLayout.java
@ -291,7 +291,7 @@ public class MachineInventoryLayout {
}
/**
justliliandev (Migrated from github.com) commented 2022-12-29 13:39:33 +00:00
Copy link

fix javadoc to imply multiple slots

fix javadoc to imply multiple slots
dphaldes (Migrated from github.com) reviewed 2023-01-06 12:49:53 +00:00
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +1,177 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2023-01-06 12:49:53 +00:00
Copy link

Just change sameItem() to isSameItemSameTags() ?

Just change `sameItem()` to `isSameItemSameTags()` ?
justliliandev (Migrated from github.com) reviewed 2023-01-06 13:01:01 +00:00
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +1,177 @@
package com.enderio.machines.common.blockentity;
justliliandev (Migrated from github.com) commented 2023-01-06 13:01:01 +00:00
Copy link

Yep

Yep
dphaldes (Migrated from github.com) reviewed 2023-01-06 13:06:56 +00:00
src/machines/java/com/enderio/machines/common/io/item/MachineInventoryLayout.java
@ -291,7 +291,7 @@ public class MachineInventoryLayout {
}
/**
dphaldes (Migrated from github.com) commented 2023-01-06 13:06:55 +00:00
Copy link

I don't understand. Can you reiterate ?

I don't understand. Can you reiterate ?
dphaldes (Migrated from github.com) reviewed 2023-01-06 13:08:17 +00:00
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +88,4 @@
public void serverTick() {
tryCraft();
super.serverTick();
processOutputBuffer();
dphaldes (Migrated from github.com) commented 2023-01-06 13:08:15 +00:00
Copy link

I have moved this down. Is it fine now ?

I have moved this down. Is it fine now ?
justliliandev (Migrated from github.com) reviewed 2023-01-06 13:12:17 +00:00
src/machines/java/com/enderio/machines/common/io/item/MachineInventoryLayout.java
@ -291,7 +291,7 @@ public class MachineInventoryLayout {
}
/**
justliliandev (Migrated from github.com) commented 2023-01-06 13:12:03 +00:00
Copy link

Some of the builder methods have javadoc that says adds a ghost slot (so singular) while having an int parameter for the amount of fields. Change the javadoc of those methods to plural

Some of the builder methods have javadoc that says adds a ghost slot (so singular) while having an int parameter for the amount of fields. Change the javadoc of those methods to plural
dphaldes (Migrated from github.com) reviewed 2023-01-06 13:20:05 +00:00
src/machines/java/com/enderio/machines/common/io/item/MachineInventoryLayout.java
@ -291,7 +291,7 @@ public class MachineInventoryLayout {
}
/**
dphaldes (Migrated from github.com) commented 2023-01-06 13:20:04 +00:00
Copy link

Understood. input and output slots have them. Storage and ghost don't. Should I change them as well ?

Understood. input and output slots have them. Storage and ghost don't. Should I change them as well ?
justliliandev (Migrated from github.com) reviewed 2023-01-06 13:22:16 +00:00
src/machines/java/com/enderio/machines/common/io/item/MachineInventoryLayout.java
@ -291,7 +291,7 @@ public class MachineInventoryLayout {
}
/**
justliliandev (Migrated from github.com) commented 2023-01-06 13:22:16 +00:00
Copy link

Yeah, all things that allow multipls slots should be written in plural

Yeah, all things that allow multipls slots should be written in plural
dphaldes (Migrated from github.com) reviewed 2023-01-06 14:38:09 +00:00
src/machines/java/com/enderio/machines/common/blockentity/CrafterBlockEntity.java
@ -0,0 +1,177 @@
package com.enderio.machines.common.blockentity;
dphaldes (Migrated from github.com) commented 2023-01-06 14:38:09 +00:00
Copy link

I have changed the type. The old method calls seem to work fine (changed the required ones). Let me know if I need to make more changes

I have changed the type. The old method calls seem to work fine (changed the required ones). Let me know if I need to make more changes
justliliandev (Migrated from github.com) approved these changes 2023-01-07 15:29:05 +00:00
justliliandev (Migrated from github.com) left a comment
Copy link

codewise this looks good, will do some ingame check next

codewise this looks good, will do some ingame check next
justliliandev commented 2023-01-07 15:56:50 +00:00 (Migrated from github.com)
Copy link

do you want to wait for slotaccess to be merged into and rebase your stuff onto it or should we merge this and I'll change the crafter to use slot access

do you want to wait for slotaccess to be merged into and rebase your stuff onto it or should we merge this and I'll change the crafter to use slot access
dphaldes commented 2023-01-07 18:18:36 +00:00 (Migrated from github.com)
Copy link

This PR is not ready to be merged as of now (I have few todos on the top. ) Also I will like to wait for slotaccess and hopefully IO config as well

This PR is not ready to be merged as of now (I have few todos on the top. ) Also I will like to wait for slotaccess and hopefully IO config as well
justliliandev (Migrated from github.com) approved these changes 2023-01-31 08:42:04 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
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
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#99
No description provided.
Delete branch "dev/crafter"

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?