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.

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

Add disable_descend group to disable active descend in climbable or liquid node #11377

Merged
merged 2 commits into from Jun 16, 2023

Conversation

Wuzzy2
Copy link
Contributor

@Wuzzy2 Wuzzy2 commented Jun 22, 2021

Fixes #11452

This PR adds the group disable_descend to disable players from actively climbing down / moving down in climbable or liquid nodes when you hit the Sneak/Aux1 key.

This is the logical counterpart to disable_jump, which disables climbing/moving upwards instead. I thought it was strange you could disable climbing upwards, but the other way.

How to test

Check out the new climbable nodes in DevTest.

@rubenwardy rubenwardy added Roadmap: Needs approval The change is not part of the current roadmap and needs to be approved by coredevs beforehand. @ Script API Feature ✨ PRs that add or enhance a feature labels Jun 22, 2021
@rubenwardy
Copy link
Member

Unfortunately, this feature is not on the roadmap and hasn't received a concept approval. Under the new rules, non-roadmap non-bugfix PRs have 7 days to be concept approved, otherwise they should be closed. These PRs can also have preapproval if they fix an issue that is concept approved

If a core dev wants to support this PR, they may reopen it and apply the "Supported by core dev" label

@rubenwardy rubenwardy closed this Jul 14, 2021
@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jul 14, 2021

What a great way to discourage community members from contributing ever to Minetest again! Nobody even looked at this PR and it is already dead. Contributing to Minetest is so frustrating.

To be honest, this is a really shitty policy. Unless your goal is to get the number of contributors down to 0 as fast as possible.

@SmallJoker SmallJoker removed the Roadmap: Needs approval The change is not part of the current roadmap and needs to be approved by coredevs beforehand. label Jul 14, 2021
@SmallJoker
Copy link
Member

Well actually this is a very simple implementation and might open up some new possibilities for ITB-style puzzles.

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jul 15, 2021

Thank you, SmallJoker. Does this count as a concept approval? If yes, then please add the "supported by core dev label".

@SmallJoker SmallJoker added the Concept approved Approved by a core dev: PRs welcomed! label Jul 15, 2021
@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jan 10, 2022

Bump.

@SmallJoker SmallJoker added the Rebase needed The PR needs to be rebased by its author. label Jan 10, 2022
@SmallJoker
Copy link
Member

Needs rebase.

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jan 10, 2022

Rebased.

These are the DevTest nodes to test:

  • testnodes:liquid_nodescend (new)
  • testnodes:climbable_nodescend (new)
  • testnodes:liquid_nojump
  • testnodes:climbable
  • testnodes:climbable_nojump
  • testnodes:climbable_move_resistance_4

@SmallJoker SmallJoker removed the Rebase needed The PR needs to be rebased by its author. label Jan 11, 2022
@Zughy
Copy link
Member

Zughy commented May 4, 2022

Uhm.. I personally don't understand what's the use of a non descendible liquid. Like.. it's a liquid. I think the real problem is disable_jump impeding people to climb and swim upwards: a good game designer wouldn't put ladders when they don't want you to climb. Imho this only makes things messier, hence 👎

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented May 9, 2022

"makes things messier" is a terrible argument if not clarified. That is highly subjective, plus, did you look at the code? It's just a very few lines of code, most of the code added for this PR is just DevTest stuff, not the actual implementation. If even this small change already is too messy, then you can use this argument as justification to reject everything.

Mainly, this change is for consistency. If there is a way to block the upwards movement, it only makes sense to also block the downwards movement. Unsurprisingly, the code for this is very simple.

I think the real problem is disable_jump impeding people to climb and swim upwards

That's not a problem at all. It works exactly as intended and never caused any trouble. Nor do I think my PR does any harm at all.

Some use cases:

  • Mystery goo: You can walk on it and don't sink in, but if you walk in from the side, you can rise
  • Force field: Blocks that only allow you to move through in one direction (possibly for some puzzle game)

And if you think that's too esoteric, I don't care. :P Minetest is more than just Minetest Game.

@MisterE123
Copy link
Contributor

MisterE123 commented May 10, 2022

while I agree with Zughy that the 'disable jump' feature of liquids is a bad design, we are now stuck with that design. this PR is a logical next step in that design, and so, unless we are going to get rid of the disable jump liquid feature and implement disabling jump in another way, we should go ahead with this.

if you can disable jump in a node, you should also be able to disable decending.

Note that no one will actually be using this feature for literal liquids. But as we know, liquid node features are used for much more than liquids. Lets either redesign the whole thing, or do this. Doing this is much easier.

An immediate usecase of this feature is a node that lets you move thru it, but not move up or down in it. (suspension field?) that would be great for puzzle maps. you could extend this to add player velocity upwards to make an antigravity beam.

there are valid usecases for this. lets not do the:

"use-case does not make sense to me so it must be useless"
classic devs``` - quote from greenxenith on discord recently

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jan 16, 2023

Fixes #11452.

(posted to force a link to appear at that issue)

@Zughy
Copy link
Member

Zughy commented Feb 21, 2023

@Wuzzy2 rebase needed

@Zughy Zughy added the Rebase needed The PR needs to be rebased by its author. label Feb 21, 2023
@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Feb 21, 2023

Rebase completed. I have no right to remove the label.

@Zughy Zughy removed the Rebase needed The PR needs to be rebased by its author. label Feb 21, 2023
@SmallJoker SmallJoker added the Rebase needed The PR needs to be rebased by its author. label May 27, 2023
@SmallJoker
Copy link
Member

Checked my pending reviews and I seem to totally have forgotten about this one.
Sorry - apparently the PR needs a rebase again.

@Wuzzy2
Copy link
Contributor Author

Wuzzy2 commented Jun 3, 2023

Rebase completed.

@Zughy Zughy removed the Rebase needed The PR needs to be rebased by its author. label Jun 3, 2023
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Works

Copy link
Member

@Desour Desour left a comment

Choose a reason for hiding this comment

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

Tested, works. LGTM. 👍

@Desour Desour merged commit 8e1af25 into minetest:master Jun 16, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Concept approved Approved by a core dev: PRs welcomed! Feature ✨ PRs that add or enhance a feature @ Script API >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to disable descending in climbable or liquid nodes
6 participants