Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for VoidProtection of Items to MultiSmelter #3650

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

Worive
Copy link

@Worive Worive commented Dec 14, 2024

I may lack context as I'm an early LV player, so I'll describe quickly what I've done (to be sure I'm missing nothing):

For each possible recipe, I'll check the max possible parallel according to the VoidProtectionHelper, and limit them according to it. If after checking each recipe I end up with an empty output (so everything at 0 parallel), I assume it's because there's not enough space.

Close GTNewHorizons/GT-New-Horizons-Modpack#17086

@Dream-Master Dream-Master requested a review from a team December 14, 2024 14:38
@serenibyss serenibyss added the enhancement Improve an existing mechanic. Please explain the change with a before/after comparison. label Dec 14, 2024
Copy link
Contributor

@HoleFish HoleFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "void protection" doesnt provide void protection. It will void stuffs if output bus is about to be full; it only provide protection when output bus is full already.

@Worive
Copy link
Author

Worive commented Dec 15, 2024

This "void protection" doesnt provide void protection. It will void stuffs if output bus is about to be full; it only provide protection when output bus is full already.

@HoleFish I did not manage to reproduce any voiding of materials, but I've noticed I missed a miss calculation of remaining cost, which would make it do 32x stone then 15x stone then 15x glass (3 steps), instead of 32x stone then 15 stone and 15x glass (2 steps), I'll push this fix, but I would gladly like if you could tell me how did you void items 🙏

In my setup, I'm using a LV energy hatch with Nichrome Coil Blocks, with 1 input hatch and 1 output hatch, but I've also tried with 2 output hatches.

@HoleFish
Copy link
Contributor

2 random types of smeltable item in input bus and a ULV output bus. Multismelter reasonably start processing and void one of them with this void protection enabled

@Worive
Copy link
Author

Worive commented Dec 15, 2024

2 random types of smeltable item in input bus and a ULV output bus. Multismelter reasonably start processing and void one of them with this void protection enabled

@HoleFish Alright, I noticed I was only trying with same item slot voiding kind (test 2 -> 5), and now I fixed the case of test 1 which is yours (somehow did not happen to me to test this situation, sorry). I've put down all the tests I've been doing to be sure I did not forget about another situation

Context: Basic Multi Smelter with a LV Energy Hatch and Nichrome Coil Blocks, with 1 input and 1 output bus and I smelt sand and cobblestone to have two types of output.

Test 1 - 2 Items with one slot (HoleFish's test)

  • Output bus with 1 available slot
  • Input bus with 1x of sand and 1x of cobblestone

Process

  1. 32x Glass
  2. 15x Glass, 17x Stone
  3. 30x Stone

Test 2 - Simple one output

  • Output bus with 1 empty slot available
  • Input bus with two stacks of sand

Process

  1. 32x glass
  2. 32x glass
  3. Not enough space

Test 3 - Non-stack output available

  • Output bus with 1 slot with 1 glass, the rest with cobblestone
  • Input bus with two stacks of sand

Process

  1. 32x glass
  2. 31x glass
  3. No enough space

Test 4 - Testing normal functioning with multiple recipes

  • Output bus with 1x glass, 1x stone, and two slots of cobblestone
  • Input bus with 64x of sand and 64x of cobblestone

Process

  1. 32x stone
  2. 1x glass, 31x stone
  3. 32x glass
  4. 30x glass

Test 5 - Testing non-stack with multiple recipes

  • Output bus with 17x stone, 17x sand, and two slots of cobblestone
  • Input bus with 64x of sand and 64x of cobblestone

Process

  1. 32x Glass
  2. 15x Glass, 17x Stone
  3. 30x Stone

@Dream-Master Dream-Master requested a review from a team December 15, 2024 12:22
@Worive
Copy link
Author

Worive commented Dec 15, 2024

Current known issues:

  • Batch mode's time is not adapted to the limited output rate by VoidProtectionHelper
  • Some configurations of VoidProtectionHelper will give incorrect getMaxParallel() results (wrong usage of VoidProtectionHelper surely)

Everything working well now ✅

Worive and others added 4 commits December 15, 2024 18:10
Allow the helper to calculate with items given as if they were already in the output buses, without having to move them in such bus to be able to perform a check
Use the new virtual item outputs in bus hability
@Worive Worive requested a review from HoleFish December 16, 2024 17:17
@Worive
Copy link
Author

Worive commented Dec 16, 2024

Ended up adding the ability to the VoidProtectionHelper to use virtual output bus items, to properly check for cases like multi-smelter.

Seems to be the more fitting solution and can be used in the future by any other machine requiring it

@HoleFish
Copy link
Contributor

2 stacks of sand, ULV output bus
result: 128 glass processed and 64 voided

@Worive
Copy link
Author

Worive commented Dec 16, 2024

As it it would be so simple to implement virtual stacks output to VoidProtectionHelper, fixed that and did all my 5 tests listed above, all working 👍 (in both normal and batch mode)

@Worive Worive requested a review from HoleFish December 16, 2024 18:39
@HoleFish
Copy link
Contributor

Stocking input bus, 192 random ores + infinite sand, LV output bus
result: 192 ingots + 192 glass processed, 128 glass voided

@Worive
Copy link
Author

Worive commented Dec 16, 2024

Ready for another checkup 👍

int maxParallelForOutput = remainingCost;

// Initialize void protection for the current output
if (protectsExcessItem()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still suggest you move this void protection part and merge into Calculate parallel part, so that subtick energy cost will be correct

@Dream-Master
Copy link
Member

@Worive any news?

@Worive
Copy link
Author

Worive commented Jan 8, 2025

@Worive any news?

Sorry I only talked about it on discord, forgot to mention it here too:

I apologize, but I don't feel like I'm able to finish this task, just looking at the code regarding what is left to fix simply bricks my mind and prevents me from thinking further. I would greatly appreciate it if someone with more experience could finish what is left of it and explain their thinking while doing it 🙏

Wish I could have finished this task by myself 😞

@Dream-Master
Copy link
Member

@HoleFish can you help here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve an existing mechanic. Please explain the change with a before/after comparison.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow void protection in multi-smelter
4 participants