Refactor controller insert code to be cleaner and do less work #1284
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While looking at item voiding bugs, etc, i noticed the logic around inserts into controllers seemed in need of a bit of a refactor.
The way it is currently structured does tests a bunch of times on the same drawers, can't exit early because of how it marks drawers for rebalancing, etc.
This refactors the code to hopefully be clearer, cleaner, and does strictly the same or less work than the old code.
For primary records:
First we gather the valid non-empty drawers that meet the predicates, and drawers that would need rebalancing if we place an item.
Then we go over these drawers , once with the strict check, and if needed, with the loose check.
If we placed an item in a balance-fill drawer, we mark the need to rebalance all the balance-fill drawers.
If we run out of items to place, we stop.
If we still have to place anything - we walk the drawer slots, and place items into the enabled ones.
Note that the latter loop did not ever rebalance drawers marked as balanced-fill even if items were placed. I believe this is a bug in theory (if primaryrecords is null, we will never rebalance), and it is easy to fix, but i left the logic as-is for now until someone smarter than me thinks about it :). Happy to fix it if it is a bug.
The split diff is much easier to read than the unified.