Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rope Ladders Update #881

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Rope Ladders Update #881

merged 5 commits into from
Sep 22, 2023

Conversation

MichaelModulo
Copy link
Member

@MichaelModulo MichaelModulo commented Jun 10, 2023 •
edited
Loading

  • Replaced villager with interaction entity.
  • Ladder can now extend into any replaceable block.
  • Players in adventure mode can no longer extend ladders.
  • Fixed bug related to incorrect ladder stack being extended.

@Bloo-dev Bloo-dev added needs-testing Requires in-game testing quality-update Improves efficiency or structure without affecting functionality of a module labels Jun 12, 2023
Bloo-dev
Bloo-dev reviewed Jun 12, 2023
View reviewed changes
gm4_rope_ladders/data/gm4_rope_ladders/tags/blocks/water.json Outdated
Comment on lines 1 to 6
{
"values": [
"water",
"bubble_column"
]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this not use #gm4:water instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know that was a block tag that existed.

SpecialBuilder32
SpecialBuilder32 requested changes Jun 26, 2023
View reviewed changes
Copy link
Member

@SpecialBuilder32 SpecialBuilder32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Only comment I have is that while standing above its difficult to target the interaction entity. Perhaps the height could be adjusted to be the full block heignt?

image

@MichaelModulo
Copy link
Member Author

I made the hitbox extend all the way up, and even slightly above the top for easier interaction.
image

Bloo-dev
Bloo-dev approved these changes Aug 12, 2023
View reviewed changes
Copy link
Member

@Bloo-dev Bloo-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this in-game and everything still works as expected.

I found some non-blocking issues that we should deal with in the near future, or in this PR if there's enough time before this is merged:

  • when placing ladders, the setblock mode is currently replace, which means that tall grass or seagrass simply disappear, without any breaking sound or particles. Simply setting this to destroy would fix this.
  • the block tag gm4_rope_ladders:valid_replace only contains gm4:replaceable and is there somwhat pointless. This could be removed.

@Bloo-dev
Copy link
Member

@SpecialBuilder32 please verify that your hitbox concerns have been resolved.

@Bloo-dev Bloo-dev added tested Has passed in-game testing and removed needs-testing Requires in-game testing labels Aug 12, 2023
TheThanathor
TheThanathor approved these changes Sep 9, 2023
View reviewed changes
Copy link
Member

@TheThanathor TheThanathor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two tiny comments, but nothing that's very important to fix

gm4_rope_ladders/data/gm4_rope_ladders/tags/blocks/valid_replace.json Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file isn't used anywhere and could be removed

...e_ladders/data/gm4_rope_ladders/functions/mechanics/ladder_placement/place_ladder.mcfunction
# place ladder
execute unless block ~ ~ ~ water run clone ~ ~1 ~ ~ ~1 ~ ~ ~ ~
execute if block ~ ~ ~ water run function gm4_rope_ladders:mechanics/ladder_placement/place_water_ladder
execute unless block ~ ~ ~ #gm4:water run clone ~ ~1 ~ ~ ~1 ~ ~ ~ ~
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically this means that if the ladder above is waterlogged it will incorrectly copy a waterlogged ladder. However the only method I found to do this is by replacing light blocks (as they are replaceable but stop water flow), so it's not a big deal.

@Bloo-dev Bloo-dev merged commit c39d15f into Gamemode4Dev:master Sep 22, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 22, 2023
@MichaelModulo
🚀 c39d15f Rope Ladders Update (#881)
c043af6 
* update rope ladders

* use gm4 water block tag

* make hitbox larger, prevent back placement

* uses destroy for particles, uses gm4:replaceable

* remove unused file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheThanathor TheThanathor TheThanathor approved these changes

@Bloo-dev Bloo-dev Bloo-dev approved these changes

@SpecialBuilder32 SpecialBuilder32 SpecialBuilder32 approved these changes

Assignees
No one assigned
Labels
quality-update Improves efficiency or structure without affecting functionality of a module tested Has passed in-game testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MichaelModulo @Bloo-dev @SpecialBuilder32 @TheThanathor