FarmersDelight icon indicating copy to clipboard operation
FarmersDelight copied to clipboard

Basically satisfied #432: Made ropes collectable from top

Open RaymondBlaze opened this issue 3 years ago • 1 comments

This PR solved #432 : Ropes now can be collected from top using Shift + Right Click.

Additional information:

  1. Player must have its mainhand empty to do this.
  2. Collected ropes will be merged into existing rope item stack in inventory (if present) or added into the first empty slot in inventory (except for both hands, because having rope item in offhand leads to the action of placing it, which is unexpected).
  3. I didn't check whether the clicked rope block is the upper end. From my POV, one can pull ropes from its middle for sure, and this function would be too useless if we add such restriction.
  4. The idea of collecting ropes from bottom is unrequired by the issue, and I haven't got any reason to do so.

RaymondBlaze avatar Mar 25 '22 10:03 RaymondBlaze

Hi there! Thank you very much for contributing this feature. ^^

You sorta nailed what I wanted to achieve with it: points 1 and 2 are solid, and 3 seems fair to avoid a pointless restriction. As for 4, bottom-end reeling would be a lot more useful if ropes collapsed without support (something I haven't yet managed to add), so it's fine without it.

I only have a few revisions after playing around with the code, mostly for optimization.

vectorwing avatar Mar 30 '22 03:03 vectorwing

Hi again! Sorry for the delay.

I decided to come back to this PR and test it again, regarding the discussion we had about leaving a hotbar slot free when reeling ropes. Turns out that the getInventory().add() method takes this into account when selecting which slot the new item goes to.

If the only empty hotbar slot is the selected slot, it will instead attempt to place it in the player's inventory, only resorting to the selected slot last. I tested locally, and it kept my hand free up until my inventory was completely full, and only my empty hand was left. Here's the test, with the arrow marking my (once) empty hand:

image

vectorwing avatar Aug 31 '22 03:08 vectorwing

@LimonBlaze, the changes are great! However, since this was aimed at 1.18.1 (again, sorry for taking so long to reply), it might be a bit outdated. I can add these changes from my end and just close the PR, while crediting you as usual, if you prefer; since a lot has happened between this and the 1.2 dev branch. 😅

vectorwing avatar Aug 31 '22 03:08 vectorwing

I can add these changes from my end and just close the PR, while crediting you as usual, if you prefer;

It's fine for me, handling a PR on an outdated branch is truly not a good idea, and I made the PR just for being helpful and improve the mod :)

RaymondBlaze avatar Aug 31 '22 05:08 RaymondBlaze

Alright! Added this feature on commit 1538311. Thank you very much for the contribution! 👍

vectorwing avatar Aug 31 '22 15:08 vectorwing