[Bug] existing pedestal connections aren't taken into account when removing Range Augments

[mfrankov]

Easy to replicate: add a tier-1 augment to a pedestal so you can connect it to another pedestal just outside the base range, then remove the augment. Items will still transfer even though the visualization will clearly show the receiver is outside the red bounding-box, and you also can no longer remove that connection due to the error "Linking Failed, Out of Range".

There's three potential options I see here:

1. Leave the connection in-place, and just update attemptUpdateLink to allow removal of an existing link regardless of other conditions. This keeps part of the bug around as items can be sent outside the sender pedestals range post augment-removal, but at least you can remove the connection.
2. Update removeRange to not let you remove augments that are needed to maintain existing connections. Presumably would need some sort of messaging sent to the player as to why the augment can't be removed.
3. Update removeRange to remove any connections that are no longer valid due to the augments being removed. Presumably would need some sort of messaging to the player outlining pedestals that were unlinked (with their world position?).

[Mowmaster]

4. When pedestal picks connection for sending, run the isInRange check, if it fails, run the remove connection function.

Should probably also do this if there is no block pedestal at location either (can't remember if that's working or not already)

[mfrankov]

My thought on keeping this inside removeRange and leaning towards option 3 would be to minimize the amount of work done during transfer (since transfer can be sped up to be once a tick).

This seems relatively safe as there are two reasons why an existing connection can become invalid due to range:

1. Removal of a range augment, so checking/invalidating the connection as part of the explicit player action of removing an augment would catch this.
2. Updates to the AugmentTieredRange config values to shorten the amount of additional range provided by those augments (as the amount of range provided is explicitly looked up and computed based on the config every time it is needed).

I figure if there was a concern around invalidation due to an update of the config, that could be part of issue 205 "Rework: Tiered Augments", since having the augments store the value they are augmenting by as nbt within the item itself would avoid this entirely. That does have the downside of having to have players re-craft/potentially go through existing pedestals to update their augments if config is changed, but that's a decision that can be made between ease of allowing config tweaks versus these sorts of edge case connection errors.

Should probably also do this if there is no block pedestal at location either (can't remember if that's working or not already)

Yup you already ignore if the destination position is no longer a pedestal in transferAction. I'm less clear if you intended to actually permanently sever the connection if that was the case (that happens within canSendToPedestal which also checks if the position is a pedestal but isn't called on non-pedestals in the current code). I can fix it to always sever the bad connection if that was the intent.

I'm happy to pick up this issue either way, so let me know if you'd prefer any of the particular options.

[Mowmaster]

i went the EZ way out on this one, ef969ac

[mfrankov]

That won't break the existing link if it's out-of-range, it will simply ignore sending for that pass (which perhaps was intentional?). You'd have to add this to the test above, i.e.:

if(level.getBlockEntity(posReceiver) instanceof BasePedestalBlockEntity pedestal && isPedestalInRange(posReceiver))

as it's the else case corresponding to that existing if that permanently severs the connection.

[Mowmaster]

yeah i dont intend to break the link because of the case where someone using tier1 range might need to swap it out for tier 4 range, a max speed pedestal would act quick enough where removing the link could be annoying as it would need to be setup again.

removing links only in the case where a pedestal block isnt present in the block location is the only time i would want to remove a link i think