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

[WIP] initial change for fixing round robin voiding` #2601

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jkieberking
Copy link

@jkieberking jkieberking commented Aug 29, 2024

What

This PR is meant to fix the voiding bug seen in #2548 where conveyors with the round robin with priority IO mode voids items.

Implementation Details

The root cause of the bug is explained here, which is essentially that we don't keep track of how much space is left in a machine/inventory when we do a dry run, so sequential dry runs think that they can insert more than they actually can (which results in us extracting/voiding the excess amount.)

Outcome

The initial version of this MR opts to keep track of what we are inserting during a dry run via a simulatedRemainingCapacityRoundRobin, which is a Object2IntMap<FacingPos>. I also updated some variable names to better show what they do.

This initial implementation does not fully work - I was hoping to push it and get some feedback/assistance as I'm new to the repo/modding GT in general, so am not sure about certain standards/etc...

The main question I have is: What is the best way to get the free input inventory space in a machine/block/chest/etc...?
- currently I am doing this via:
```
int freeInventoryInSubRoute = 0;
int slots = routePath.getHandler().getSlots();

            for (int i = 0; i < slots; i++) {
                int slotLimit = routePath.getHandler().getSlotLimit(i);
                ItemStack testStackToInsert = stack.copy();
                testStackToInsert.setCount(slotLimit);
                ItemStack insertedTest = routePath.getHandler().insertItem(i, testStackToInsert, true);
                ItemStack stackInSlot = routePath.getHandler().getStackInSlot(i);
                freeInventoryInSubRoute += slotLimit - insertedTest.getCount();
            }
       but this feels hacky? not sure if there is a better way (I was looking around the code, but wasn't finding anything)

## Still TODO (in addition to the above questions ^), want to finish these before this PR is fully "ready to go" (not a WIP)

- [ ] fix niche cases
- [ ] fully comment code
- [ ] create tests
- [ ] break certain blocks of code out into smaller functions

@jkieberking jkieberking requested a review from a team as a code owner August 29, 2024 21:33
Iterator<ItemRoutePath> routePathIterator = copy.listIterator();
int inserted = 0;
int count = stack.getCount();
int c = count / copy.size();
int m = c == 0 ? count % copy.size() : 0;
Copy link
Author

Choose a reason for hiding this comment

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

does anyone know what m is meant to do here? I wasn't sure when reading the code/context of the code around it

@@ -357,6 +357,7 @@ protected int moveInventoryItems(IItemHandler sourceInventory, IItemHandler targ
}
}
}

Copy link
Author

Choose a reason for hiding this comment

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

not sure how this got here, will remove it

@ALongStringOfNumbers ALongStringOfNumbers added the type: bug Something isn't working label Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants