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.
I have discovered a crash relating to inserting cards into a laserio node and decided to investigate and find a resolution for the issue.
The issue is that if you insert cards into a laserio node using any form of item transfer (either hopper, other mods, or using laserio itself), then the node will allow all 25 slots of its inventory (minus the one overclocker slot) to be filled with cards. To recreate just put a hopper/chute/etc on top of a node, and put 10 item cards in it.
Only the player should be allowed to insert into the cardholder slots of a node, not a machine, so to prevent this I edited the
LaserNodeItemHandler::isItemValid
method to disallow cards from being inserted into slots 9-25. This does not prevent players from interacting with the card holder slots themselves, just from cards being transferred by other means into those slots of the node.The other issue that contributed to this crash is that
LaserNodeBE::populateRenderList
iterates through all 25 slots, not just the 9 card slots. This means that if cards end up in any of the other 15 slots (like through the bug above) that it will callMiscTools::findOffset
with a slot parameter greater than 9, causingoffsets[slot]
to throw anOutOfBoundsException
. This is what actually crashes the game. I considered also adding a check to theMiscTools::findOffset
function to check thatslot
is in range ofoffsets.length
, but decided to leave that out. There are other parts of the code that call that function, so it may be worth adding that check in the future to prevent other obscure crashes.