New mountaineering module

[AdequateUsername]

Adds some mountaineering-related gear to the game: skis, poles and crampons.

Skis

• crafted with iron ingots in the two side columns and iron boots in the centre of CC (advancement for crafting)
• give slowness on non-snowy blocks
• give speed on snowy blocks
• give increased speed when moving downhill (advancement for reaching maximum speed boost)
• create some particles when skiing

Poles

• crafted with a stick in top centre and bottom centre and tripwire hook in the centre of CC
• need to hold in mainhand and offhand for effects
• give jump boost 1 normally, and 2 when skiing uphill; this is on a cooldown
• reduce the vertical speed threshold for getting speed boosts when skiing
• reduce fall damage when used without skis

Crampons

• crafted with chainmail boots in top centre, and iron nuggets in centre row of CC
• give slowness effects (they shouldn't be easy to walk in)
• allow climbing of vertical walls made of stone, grass block, or dirt when both hands are empty
• you cannot 'climb' on a wall indefinitely; after a while you fall off
• the climbing mechanism is not perfect because (I think) of shulkers; it is assumed that all climbers make mistakes

Remarks

• I can't see any changes needed to meet BPR's CC update, in that what I have looks similar to other modules. I might have missed something though I realise some changes are needed; going to look at this CC should all be correct now
• Orbis compatibility would be good, but it looks like it is still being updated, so I'll leave this for now

[Bloo-dev]

First of all, thank you for your PR! We like the basic principles of this module and would love to see it be a part of Gamemode 4, especially with added Orbis compatibility to make these appear in loot chests!

Our in-game tests have shown some issues and necessary feature changes that are outlined below.
We have also found some more technical issues with the code, however, these may not need to be addressed individually, as some parts of this module should receive some refactoring to improve the end user experience. Though below you can still find a couple of out suggestions; bear in mind that this is not a comprehensive code review, though.

Skies:

• the block checks need to be refined, the stepping_on condition sadly only checks the block below the player, neglecting, for example, snow layers entirely. This should be replaced with a smarter "snow detection system" which respects snow layers as a snowy surface, regardless of the block below the snow layer.
• the falling condition seems to be somewhat flawed (air 1.3 blocks below the player yields plenty of false positives). Use the scoreboard objective minecraft.custom:minecraft.fall_one_cm or advancement condition minecraft:fall_from_height instead, however, bear in mind that these only yield values once the player has landed making them less useful as it seems like what you want to track is players having a negative y-velocity (running downhill). The latter may be achieved by spawning markers and reading their y-coordinate to get a rough velocity approximation.
• instead of using potion effects, consider using attributes instead to apply speed (allows for more precise control, better stacking control)
• it would be nice if skis ramped up in speed when going downhill and if the skis emulated the slipperiness of skiing instead of the hard turns you can do when walking

Poles & Crampons:

• Whereas the skis have their unique niche and have the interesting snow interaction the crampons & poles seem to be lacking a bit. We suggest spicing them up quite a bit to make them useful, here are our suggestions:
  • Turn the crampons into boots that let you climb vertical walls by placing temporary Shulkers as temporary platforms.
  • Rework the poles -- tripwire hooks can't be used as the player can place them down. Instead, use an unplaceable item and increase the synergy with the skis (making you speed up quicker?). These should be early-game to give them the most use.

Recipes

The current recipes seem a bit awkward and could use a bit of a re-vamp. Our points of concern are:

• iron pressure plates seem weird in a recipe
• dripstone seems weird in a recipe
  Take some inspiration from our latest recipe re-vamp for custom crafters in which we fixed some of our old recipes.
  This PR will also have to be updated to comply with @BluePsychoRanger's latest Custom Crafters update (CC Shifting #787 ). @BluePsychoRanger will give you a detailed review as well to add more info on this topic.

Orbis Compatibility

Making these generate in Orbis Chests will advertise them to players, consider adding Orbis Compatibility. CC @kruthers who's currently updating Orbis.

[AdequateUsername]

Firstly, thank you for the feedback. A quick update, in part to prove that I'm still working on this

• in general, I've given the skis better functionality: a better snow detection mechanism, a smarter way to detect falling (calculate y-velocity) and add speed (now with attributes, not potion effects) depending on v_y.
• re: emulating slipperiness, that was something I wanted to implement from the start but can't find a nice way of doing it. The way I envision it is that it's as if the player is on an ice block. I don't know if it's possible to set the block beneath the player to ice with the texture of whatever block is actually there, but that sounds rather messy.
• adjusted recipes, and changed the poles to be sticks (maybe too 'boring'?)

The adding-speed functionality generally follows the mechanism used in speed paths, but it feels a bit clunky/I don't know if it puts an unnecessary burden lag-wise. A way to increase the speed attribute as a function of the player's y-velocity would probably be the gold standard, but the current method didn't seem horrific so I went with it.

Further updates will come for poles/crampons to make them more whole, bearing in mind Bloo's feedback. And adjusting to conform to BPR's CC update + Orbis compatibility (but I need to learn how to do that first.. :) I can't guarantee any timeframe right now; sorry for that, but I haven't forgotten this.

[TheThanathor]

Since you're still working on this I won't do a full in-depth review. On the functionality side I've tested the skis and they feel great to use now!

On the technical side you should attempt to limit the amount of execute as @a[gamemode=!spectator] at @s.
For example, after calling gm4_mountaineering:ski_effects/check_snowy_block you should put the gm4_mountaineering:get_velocity call in that function directly instead of calling it from main later.

Directly calling a function will keep any as or at context, so you only have to check for any additional conditions. This goes for pretty much all of the ski related function calls in main, where you can call from the checking function directly instead of adding a tag to the player then checking for players with that tag.

[TheThanathor]

Pretty much everything looks good, but there's some updates needed to make the module compatible with 1.20+, I'd also suggest trying to improve climbing a bit more, but that is up to you.

[AdequateUsername]

Thanks for the comments. Translations have been updated to the latest standard, and the formatting changed as requested.

Some notes about the climbing:

• I have now made a small change that (should) make it much easier to climb a height-5 wall (or any odd-height wall).
• I have kept the countdown-based shulker-killing mechanism; as I don't think players should be able to hold onto a wall forever (it's unrealistic for someone to do this).
• I think much of the remaining clunkiness will be coming from the shulker-spawn function not running during a jump (since the shulker-spawn function is running every 4t, but a jump may be finished within that). I don't think there's a way to improve that without increasing the number of times the function runs, which I'm not keen to do.

I'll continue to think of possible improvements to the climbing system over the next days, but this is where I am now.

[TheThanathor]

two minor problems, other than that it looks good!

[TheThanathor]

this means the gm4_mountaineering:climbing_effects/add_climb_effects can run twice if both checks are true, it's better to set a dummy score to 0 first, then set it to 1 if the function is called, only running the second check if the first one failed.

[AdequateUsername]

I've added unless block ^ ^ ^1 #gm4_mountaineering:climbable_blocks into the second condition - that is, only run add_climb_effects in the second check if the condition for the first check was not met. I think this should achieve the same result? (in my own test, it makes climbing a little cleaner)

[TheThanathor]

That will work as well, but using the score method is a little bit more optimized as it only does the score check if the first check succeeds, by using unless block it needs to do a block check, which is worse for performance

[AdequateUsername]

Alright, makes sense. Changed to a scoreboard approach (it seems a little clunky, so I don't know if there's a better way to do it, but I know at least that it works)

[TheThanathor]

couple more points found during the meeting

[BPR02]

The Ski Poles should be called "Ski Pole" which would make it more clear that the player needs two of them and that they need one in each hand. We also discussed making them unstackable, but that can be done later and isn't an immediate issue. Ping me on discord for info about making items unstackable.

[Bloo-dev]

We now have a planned timeline for this:

Once all the multiplayer issues (second review by @TheThanathor) and the fixes listed by @TheThanathor above have been applied, this will get one more feature-check just to make sure all the item names and Advancements follow Gamemode 4 Guidelines and be merged after.
However, the module will get a note of Experimental Module, similar to Combat Expanded.

This is due to us having ideas on how to make the skis feel better and us wanting to apply some advanced beet stuff to make the Crampons easier to use:

• The skies may be better of being a sled, similar to the sleds found in Origin Realms (but this would look very bad without the resource pack
• The ski poles should be unstackable
• Using beet and display entities the shulkers should also summon a display entity which mimics the block being climbed on, but pushed out by one pixel (creating the impression of a ledge the player stands on)

These changes are not to be made by you (unless you really want to) but will be added as a later update.

[AdequateUsername]

Okay, thanks for the update and comments; some changes have been made.

• 'Ski Poles' now called 'Ski Pole'; I haven't made these unstackable for now, but it makes sense/makes it clearer that you need to have one in each hand. Not sure of the most elegant way.
• Shulker peek setting is set by a different tag, so should be fine in multiplayer
• Velocity is updated also when holding poles now, so you can't remove skis while going uphill to keep jump boost effects
• add_climb_effects shouldn't run twice now, but I made a comment above under the original suggestion to confirm the approach makes sense

[TheThanathor]

looks good to me! There's def some opportunity to improve things with the new ride command, but that can be a thing for a later update

[AdequateUsername]

Final tweaks:

• scoreboard-based approach to prevent add_climb_effects running twice when the two possible conditions are both met
• the ski pole can no longer be stacked

[SpecialBuilder32]

Alright it looks like this is all set to merge 🎉 Thanks for all your patience and hardwork @AdequateUsername.

We've got a marketing plan laid out for the next few weeks to publicize newish modules in various discord servers: Mountaineering is slated for next week's spotlight.