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

Modular machine blockentity #173

Merged
Rover656 merged 26 commits from feature/modular-machine-blockentity into dev/1.20.1 2023-07-03 21:32:34 +00:00
Conversation 20 Commits 26 Files changed 54 +1986 -1485
Rover656 commented 2023-06-30 09:23:09 +00:00 (Migrated from github.com)
Copy link

Description

Change summary:

  • Base values for scalables are now ints
  • EnderBlockEntity#isClientSide removed for lack of safety in assuming a null level means server
  • Many null check improvements
  • Removed a bunch of base classes. Tasks are now performed using task hosts.
  • Fluid tanks are now generalized and handled by MachineBlockEntity
  • Improvements to capability caching, however this needs a future optimization pass
  • Added Non-Null (NN) variants of getInventory and getFluidTank for when you know a machine has one.
  • Moved all machine scalable base values to configs
  • Decided on a starting value for burning power generation, the config for stirling generator also powers primitive alloy smelter
  • Powered block state property is now handled by PoweredMachineBlockEntity
  • Ability to hide the energy and fluid capabilities from external blocks
  • Improved null checking across many machine implementations

Future work

  • More optimization work for capability caches
  • Work on tidying the vacuum blocks
  • Try and change how impulse hopper energy consumption works
  • Scaling for generator efficiency and output
  • A no-power recipe interface for use with CraftingMachineTask
  • Fluid support for crafting tasks (old todo)
  • A possible way to pass all of a machines inventories (items, fluids etc.) in one parameter?

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 Change summary: - Base values for scalables are now ints - EnderBlockEntity#isClientSide removed for lack of safety in assuming a null level means server - Many null check improvements - Removed a bunch of base classes. Tasks are now performed using task hosts. - Fluid tanks are now generalized and handled by MachineBlockEntity - Improvements to capability caching, however this needs a future optimization pass - Added Non-Null (NN) variants of getInventory and getFluidTank for when you *know* a machine has one. - Moved all machine scalable base values to configs - Decided on a starting value for burning power generation, the config for stirling generator also powers primitive alloy smelter - Powered block state property is now handled by PoweredMachineBlockEntity - Ability to hide the energy and fluid capabilities from external blocks - Improved null checking across many machine implementations # Future work - More optimization work for capability caches - Work on tidying the vacuum blocks - Try and change how impulse hopper energy consumption works - Scaling for generator efficiency and output - A no-power recipe interface for use with CraftingMachineTask - Fluid support for crafting tasks (old todo) - A possible way to pass all of a machines inventories (items, fluids etc.) in one parameter? # 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 -->
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:30:34 +00:00
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:31:32 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/CraftingMachineTask.java
@ -0,0 +61,4 @@
protected abstract void consumeInputs(R recipe);
protected abstract int makeProgress(int remainingProgress);
Rover656 (Migrated from github.com) commented 2023-06-30 11:31:32 +00:00
Copy link

The rationale behind "progress" is to allow machines that may not necessarily consume power to still take advantage of the crafting tasks. 90% of time, "progress" will just be "energy".

The rationale behind "progress" is to allow machines that may not necessarily consume power to still take advantage of the crafting tasks. 90% of time, "progress" will just be "energy".
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:34:07 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/CraftingMachineTask.java
@ -0,0 +140,4 @@
// region Resource Depletion
protected boolean placeOutputs(List<OutputStack> outputs, boolean simulate) {
// TODO: Handle fluids too.
Rover656 (Migrated from github.com) commented 2023-06-30 11:34:06 +00:00
Copy link

At some point I might make a "MachineStorages" class/interface or something that will allow fetching of inventories, tanks or any other kinds of resource targets, rather than having to pass the inventory, tank etc. every time individually.

At some point I might make a "MachineStorages" class/interface or something that will allow fetching of inventories, tanks or any other kinds of resource targets, rather than having to pass the inventory, tank etc. every time individually.
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:40:29 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/PoweredTaskMachineEntity.java
Rover656 (Migrated from github.com) commented 2023-06-30 11:40:29 +00:00
Copy link

This class has just been "stubbed" to use the new task host, it prove that the host had the same functionality as the original.
The idea however is for this class (and other abstract classes inheriting from PoweredMachineEntity) will be getting removed.

This class has just been "stubbed" to use the new task host, it prove that the host had the same functionality as the original. The idea however is for this class (and other abstract classes inheriting from PoweredMachineEntity) will be getting removed.
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:41:01 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/PoweredMachineEntity.java
Rover656 (Migrated from github.com) commented 2023-06-30 11:41:00 +00:00
Copy link

This was removed because it was simply not required turns out; idk who added this or why (was probably me during one of the million rewrites)

This was removed because it was simply not required turns out; idk who added this or why (was probably me during one of the million rewrites)
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:42:22 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/MachineBlockEntity.java
Rover656 (Migrated from github.com) commented 2023-06-30 11:42:21 +00:00
Copy link

I need to verify this; feels like it should actually be false.

I need to verify this; feels like it should actually be false.
Rover656 (Migrated from github.com) reviewed 2023-06-30 11:45:15 +00:00
src/machines/java/com/enderio/machines/common/blockentity/SoulBinderBlockEntity.java
Rover656 (Migrated from github.com) commented 2023-06-30 11:45:15 +00:00
Copy link

This is the most important read; this is what a converted machine will look like.

This is the most important read; this is what a converted machine will look like.
ferriarnus (Migrated from github.com) reviewed 2023-06-30 22:05:50 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/PoweredMachineEntity.java
ferriarnus (Migrated from github.com) commented 2023-06-30 22:05:50 +00:00
Copy link

Is this not the way other mods interact with our caps? This is how we expose caps no?

Is this not the way other mods interact with our caps? This is how we expose caps no?
Rover656 (Migrated from github.com) reviewed 2023-06-30 22:07:24 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/PoweredMachineEntity.java
Rover656 (Migrated from github.com) commented 2023-06-30 22:07:23 +00:00
Copy link

Capability providers are doing this for us; 9e37ca9b2a/src/core/java/com/enderio/core/common/blockentity/EnderBlockEntity.java (L237)

Capability providers are doing this for us; https://github.com/SleepyTrousers/EnderIO-Rewrite/blob/9e37ca9b2abb1728990fc890dd71fbb94ad60820/src/core/java/com/enderio/core/common/blockentity/EnderBlockEntity.java#L237
Rover656 (Migrated from github.com) reviewed 2023-07-02 01:01:45 +00:00
Rover656 (Migrated from github.com) left a comment
Copy link

Some more comments for myself.

Some more comments for myself.
src/machines/java/com/enderio/machines/common/blockentity/AlloySmelterBlockEntity.java
@ -172,2 +50,2 @@
public static final QuadraticScalable CAPACITY = new QuadraticScalable(CapacitorModifier.ENERGY_CAPACITY, () -> 100000f);
public static final QuadraticScalable USAGE = new QuadraticScalable(CapacitorModifier.ENERGY_USE, () -> 30f);
public static final QuadraticScalable CAPACITY = new QuadraticScalable(CapacitorModifier.ENERGY_CAPACITY, MachinesConfig.COMMON.ENERGY.ALLOY_SMELTER_CAPACITY);
public static final QuadraticScalable USAGE = new QuadraticScalable(CapacitorModifier.ENERGY_USE, MachinesConfig.COMMON.ENERGY.ALLOY_SMELTER_USAGE);
Rover656 (Migrated from github.com) commented 2023-07-02 00:50:33 +00:00
Copy link

Still need to move this into its own file for clarity

Still need to move this into its own file for clarity
Rover656 (Migrated from github.com) commented 2023-07-02 00:50:53 +00:00
Copy link

I think I will make this go at the same config value for the generation rate of the stirling generator.

I think I will make this go at the same config value for the generation rate of the stirling generator.
src/machines/java/com/enderio/machines/common/blockentity/base/PoweredTaskMachineEntity.java
Rover656 (Migrated from github.com) commented 2023-07-02 00:54:57 +00:00
Copy link

Might just do a hasEnergy() on the PoweredMachineEntity class

Might just do a hasEnergy() on the PoweredMachineEntity class
src/machines/java/com/enderio/machines/common/blockentity/task/IMachineTask.java
@ -0,0 +1,12 @@
package com.enderio.machines.common.blockentity.task;
Rover656 (Migrated from github.com) commented 2023-07-02 00:55:54 +00:00
Copy link

Still need to do this

Still need to do this
src/machines/java/com/enderio/machines/common/blockentity/task/SpawnTask.java
Rover656 (Migrated from github.com) commented 2023-07-02 01:01:28 +00:00
Copy link

This method will return the event rather than the (non existent) SpawnGroupData

This method will return the event rather than the (non existent) SpawnGroupData
ferriarnus (Migrated from github.com) reviewed 2023-07-02 16:49:35 +00:00
ferriarnus (Migrated from github.com) left a comment
Copy link

Just some minor questions on my first read

Just some minor questions on my first read
src/machines/java/com/enderio/machines/common/blockentity/PoweredSpawnerBlockEntity.java
@ -41,2 +46,4 @@
private final MachineTaskHost taskHost;
public PoweredSpawnerBlockEntity(BlockEntityType type, BlockPos worldPosition, BlockState blockState) {
ferriarnus (Migrated from github.com) commented 2023-07-02 16:48:59 +00:00
Copy link

Seems I have forgotten to turn the colour into a config like the vacuum blocks, but that should be done.

Seems I have forgotten to turn the colour into a config like the vacuum blocks, but that should be done.
src/machines/java/com/enderio/machines/common/blockentity/StirlingGeneratorBlockEntity.java
@ -36,10 +43,18 @@ public class StirlingGeneratorBlockEntity extends PowerGeneratingMachineEntity {
public StirlingGeneratorBlockEntity(BlockEntityType<?> type, BlockPos worldPosition,
BlockState blockState) {
ferriarnus (Migrated from github.com) commented 2023-07-02 16:26:28 +00:00
Copy link

What does the min do here, as less then 1 is 0?

What does the min do here, as less then 1 is 0?
src/machines/java/com/enderio/machines/common/blockentity/base/VacuumMachineBlockEntity.java
ferriarnus (Migrated from github.com) commented 2023-07-02 16:39:03 +00:00
Copy link

this generateParticle method should be a base method (together with the range values).

this generateParticle method should be a base method (together with the range values).
src/machines/java/com/enderio/machines/common/blockentity/task/host/MachineTaskHost.java
@ -0,0 +121,4 @@
return;
}
// Load any pending tasks.
ferriarnus (Migrated from github.com) commented 2023-07-02 16:44:47 +00:00
Copy link

This is still the same OnLoad logic, right? So it still doesn't run properly when placing a block (needed for the spawner).

This is still the same `OnLoad` logic, right? So it still doesn't run properly when placing a block (needed for the spawner).
Rover656 (Migrated from github.com) reviewed 2023-07-02 16:50:30 +00:00
src/machines/java/com/enderio/machines/common/blockentity/StirlingGeneratorBlockEntity.java
@ -36,10 +43,18 @@ public class StirlingGeneratorBlockEntity extends PowerGeneratingMachineEntity {
public StirlingGeneratorBlockEntity(BlockEntityType<?> type, BlockPos worldPosition,
BlockState blockState) {
Rover656 (Migrated from github.com) commented 2023-07-02 16:50:30 +00:00
Copy link

ah crap yea that should be a max, its just to stop the scale going to 0.

ah crap yea that should be a max, its just to stop the scale going to 0.
Rover656 (Migrated from github.com) reviewed 2023-07-02 16:50:48 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/VacuumMachineBlockEntity.java
Rover656 (Migrated from github.com) commented 2023-07-02 16:50:48 +00:00
Copy link

could you expland?

could you expland?
Rover656 (Migrated from github.com) reviewed 2023-07-02 16:51:05 +00:00
src/machines/java/com/enderio/machines/common/blockentity/PoweredSpawnerBlockEntity.java
@ -41,2 +46,4 @@
private final MachineTaskHost taskHost;
public PoweredSpawnerBlockEntity(BlockEntityType type, BlockPos worldPosition, BlockState blockState) {
Rover656 (Migrated from github.com) commented 2023-07-02 16:51:04 +00:00
Copy link

I'd leave that to a future PR, this one's scope is already really wise

I'd leave that to a future PR, this one's scope is already really wise
Rover656 (Migrated from github.com) reviewed 2023-07-02 16:51:16 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/host/MachineTaskHost.java
@ -0,0 +121,4 @@
return;
}
// Load any pending tasks.
Rover656 (Migrated from github.com) commented 2023-07-02 16:51:16 +00:00
Copy link

Yeah it is, whats not working exactly?

Yeah it is, whats not working exactly?
ferriarnus (Migrated from github.com) reviewed 2023-07-02 16:53:56 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/VacuumMachineBlockEntity.java
ferriarnus (Migrated from github.com) commented 2023-07-02 16:53:56 +00:00
Copy link

Currently, you can find the same method in all classes that have a range (vacuum blocks and powered spawner). It would be better to place it in the base machine class with a getRange method (and storing of that range in nbt) that these classes can overwrite. As to not reimplement it every time.

Currently, you can find the same method in all classes that have a range (vacuum blocks and powered spawner). It would be better to place it in the base machine class with a getRange method (and storing of that range in nbt) that these classes can overwrite. As to not reimplement it every time.
ferriarnus (Migrated from github.com) reviewed 2023-07-02 16:55:19 +00:00
src/machines/java/com/enderio/machines/common/blockentity/task/host/MachineTaskHost.java
@ -0,0 +121,4 @@
return;
}
// Load any pending tasks.
ferriarnus (Migrated from github.com) commented 2023-07-02 16:55:19 +00:00
Copy link

that OnLoad is called before Load when placing a block, making it so the BlockEntityData with the entitytype is empty when the task is created. After adding power the task gets remade and is fine.

that OnLoad is called before Load when placing a block, making it so the BlockEntityData with the entitytype is empty when the task is created. After adding power the task gets remade and is fine.
ferriarnus (Migrated from github.com) reviewed 2023-07-02 16:56:12 +00:00
src/machines/java/com/enderio/machines/common/blockentity/StirlingGeneratorBlockEntity.java
@ -36,10 +43,18 @@ public class StirlingGeneratorBlockEntity extends PowerGeneratingMachineEntity {
public StirlingGeneratorBlockEntity(BlockEntityType<?> type, BlockPos worldPosition,
BlockState blockState) {
ferriarnus (Migrated from github.com) commented 2023-07-02 16:56:12 +00:00
Copy link

Yeah that makes more sense

Yeah that makes more sense
Rover656 (Migrated from github.com) reviewed 2023-07-02 20:45:13 +00:00
src/machines/java/com/enderio/machines/common/blockentity/base/VacuumMachineBlockEntity.java
Rover656 (Migrated from github.com) commented 2023-07-02 20:45:13 +00:00
Copy link

I'm going to say that that's a future PR thing; this PR's scope is already pretty wide

I'm going to say that that's a future PR thing; this PR's scope is already pretty wide
justliliandev (Migrated from github.com) approved these changes 2023-07-03 20:55:01 +00:00
justliliandev (Migrated from github.com) left a comment
Copy link

looks good

looks good
src/machines/java/com/enderio/machines/common/block/MachineBlock.java
@ -74,22 +75,29 @@ public class MachineBlock extends BaseEntityBlock {
private void updateBlockEntityCache(LevelReader level, BlockPos pos) {
justliliandev (Migrated from github.com) commented 2023-07-03 20:01:57 +00:00
Copy link

don't do a blind cast

don't do a blind cast
src/machines/java/com/enderio/machines/common/blockentity/base/VacuumMachineBlockEntity.java
@ -0,0 +36,4 @@
public VacuumMachineBlockEntity(BlockEntityType<?> pType, BlockPos pWorldPosition, BlockState pBlockState, Class<T> targetClass) {
super(pType, pWorldPosition, pBlockState);
this.targetClass = targetClass;
add2WayDataSlot(new IntegerDataSlot(this::getRange, this::setRange, SyncMode.GUI));
justliliandev (Migrated from github.com) commented 2023-07-03 20:42:35 +00:00
Copy link

rename clazz to targetClass

rename clazz to targetClass
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
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#173
No description provided.
Delete branch "feature/modular-machine-blockentity"

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?