gnembon / scarpet

Public Repository of scarpet programs for Minecraft
Creative Commons Zero v1.0 Universal
351 stars 157 forks source link

Rope ladder update #329

Closed opsaaaaa closed 1 year ago

opsaaaaa commented 1 year ago

I wanted to try this app on my server, sorry I kinda rewrote the whole thing.

notable changes:

Ghoulboy78 commented 1 year ago

IMO some people probably liked the fact that you could have unsupported ladders. IMO it's best to have that in a variable like global_break_unsupported_ladders and have that false to emulate original behaviour. Then let the person downloading the app decide for themselves whether or not they want to keep or break unsupported ladders.

Ghoulboy78 commented 1 year ago

whoops

opsaaaaa commented 1 year ago

@Ghoulboy78 thanks for looking at this. I'll add some fixes probably sometime this week.

opsaaaaa commented 1 year ago

@Ghoulboy78

I agree that some players probably liked the unsupported ladders. And come to think about it, breaking unsupported ladders is not actually what i wanted anyway. So I propose something else instead.

As a player standing next to a rope ladder. I want to be able to dismantle the rope ladder as easily as I deployed it.

Given that I am holding shift When I break a ladder It should also break vertically connected ladders.

opsaaaaa commented 1 year ago

I also added easy_pickup and sky_ropes features

Given I am holding shift And my ladder has bottomed out When I place the ladder Then it should extend the ladder into the sky.

As a player at the top of a cliff with a rope hanging down When I dismantle the ladder Then It the ladder items should teleport to me

opsaaaaa commented 1 year ago

I found and fixed a bug with useup_item function. It was setting the first hotbar slot instead of the selected_slot. Its working correctly now, and I added the place ladder sound.

altrisi commented 1 year ago

Only one thing from my testing, my ladders float after breaking supporting ones, unless sky_ladders is off.

opsaaaaa commented 1 year ago

@altrisi I'll be totally honest. I cant guarantee I'll get to fixing that. Probably not tbh. If i do, it will be a few months.

altrisi commented 1 year ago

Makes sense, we've been exceptionally slow (well, sadly honestly not sure if exceptionally). I'll test it back later and if it's not a regression I'll approve it, and just make an issue for improvement, that way it'll only be missing one review, which ig is better.

opsaaaaa commented 1 year ago

I updated the description to reflect the changes. I should have done that initially.

I've also been running this on a server with friends for like 8-10 months now. No real issues with it. So its had lots of in game play testing.

I think this could probably be easily merged. unless there is some other issue with it.

@Ghoulboy78

Ghoulboy78 commented 1 year ago

Also, if you got a notification from another account, that's cos I was trying to review while logged onto another account, so oops...