Add rotation support for wallmounted nodes in 'ceiling' or 'floor' mode

[Wuzzy2]

This PR extends the paramtype2 wallmounted (and colorwallmounted) by adding variants of the ceiling and floor version of the node, both rotated by 90°.

Motivation

It always bothered me that floor/ceiling signs in MTG cannot be rotated, they were oriented always to the same direction when on the floor. But this is not only about MTG, any other wallmounted nodes that you could place on the floor (like buttons, or knobs) have this problem as well.

Unused values in wallmounted

wallmounted has 3 bits, and 8 possible values. Currently, only 6 values are used (one for each direction). So there are 2 values remaining. My idea is to use these 2 values for rotation, which seems perfect.

The floor and ceiling nodes were chosen since those seem to be the most relevant cases where a rotation makes sense. When you look at a floor sign, you could look at it from different direction, and it always looks ugly when it faces the wrong way.

Unfortunately, due to the data limit, only two out of 4 possible facing directions are possible. I chose a 90° rotation because this is exactly what is needed for stuff like MTG signs or buttons. This is because those things are (mostly) symmetrical on the horizontal axis, therefore it does not matter if you look at them from the front or back, but it DOES make a difference if you look at them from the side. Therefore, the 90° rotation is the most reasonable IMO on floor and ceiling.

The new wallmounted is useful when it does not matter if you are looking at the thing from the front or backside. Note that there will still be nodes when this is not enough; just use facedir. But I think it is still a nice to not let get any bit go to waste. :D

Why not a new paramtype2?

Because I programmed it so to be a simple extension of wallmounted, with backwards-compability in mind. Adding a full-blown new paramtype2 might be too much, and IMHO also would not make a lot of sense anyway because the "old" wallmounted paramtype2 which doesnt support 90° rotations will still have two completely useless param2 values

Why not letting games figure it all out on their own?

In fact, I tried exactly that a long time ago to do this in MineClone 2 with the buttons, and in theory it would have been possible … kinda. However, I quickly noticed the code for that would become VERY convoluted for a relatively simple task, and I eventually gave up. There are some difficulties with a pure mod-based approach, some of which cannot be worked around at all:

• No client prediction
• Your only chance to get anything done is by using facedir, but this paramtype2 doesn't support attached nodes
• You have to overwrite the placement handler which significantly increases the game code complexity for something that should be simple to do
• You have less coloring options with colorfacedir than with colorwallmounted

So I'm pushing for an engine change.

Use cases

• A floor or ceiling sign (like in MTG) that can also be rotated to the side
• A floor button (like in MCL2) that can be rotated to the side
• An attachable award plate
• A medieval wall shield
• A flat floor torch that can also face the other way (using torchlike)

Implementation

The param2 values 6 and 7 (after applying modulo 8, of course) now carry a special meaning. These values were unused before. If a wallmounted node has the following param2 values, this is what will happen:

• 6: The node is on the ceiling (like param2=0), but rotated 90°
• 7: The node is on the floor (like param2=1), but rotated -90°

Features:

• The drawtypes normal, nodebox, mesh, torchlike and signlike are supported
• The paramtype2 colorwallmounted should also work as expected
• attached_node=1 should work as well
• The function minetest.wallmounted_to_dir will return the correct direction for the new param2 values
• Many new test nodes for DevTest

IMPORTANT: The behavior of placing wallmounted nodes does NOT change by default. E.g. you will always get param2 of 0 or 1 (ceiling or floor) when placing a wallmounted node on the floor or ceiling, and never a 6 or 7. This means putting a sign on the floor or ceiling will always have the same rotation as before.
This was done because of backwards-compability since mods might assume that the param2 values 6 or 7 are impossible.

That's what the new node field wallmounted_rotate_vertical is for. If set to true, the node is placed with the special param2 value, if appropriate. For example, if you put a sign on the floor, it will be facing in the right direction.

Example: Minetest Game

The flexibility of this implementation becomes apparent when you look at Minetest Game. Minetest only needs to add wallmounted_rotate_vertical=true to the sign defs and BOOM! Instant rotable floor/ceiling signs. :D All the complexity and awkwardness you would have to deal with a facedir node is abstracted away from the game.

MTG should probably also update the screwdriver for obvious reasons.

Oh, and the same applies to MineClone 2's buttons and all other games that have similar things.

How to test

1. Open DevTest
2. Place a Chest of Everything
3. Place some wallmounted test nodes (there are many variants) and use the Param2 Tool to test all rotations
4. Place the “Rotatable” wallmounted test nodes on the floor or ceiling to test if wallmounted_rotate_vertical works. You can also use the Nodebox Sign and the Button.
5. Use the Falling Node Tool to test the falling node image
6. Test the client prediction of the “Rotatable” wallmounted test nodes by placing them over and over again in a slow connection
7. Test if the attached wallmounted node works as expected (Use the Param2 Tool to get the special rotation)

Note: Not all wallmounted test nodes have wallmounted_rotate_vertical set to true. This is intentional, to test if the nodes without it still behave properly.

[Wuzzy2]

Screenshot:

[Wuzzy2]

Update! I have updated this PR today to incorporate the latest changes to builtin.

[sfan5]

Discussed in meeting, concept accepted by coredevs.

[Zughy]

@Wuzzy2 please rebase

[Wuzzy2]

@Zughy2: Done.

[Zughy]

@Wuzzy2 aaand... rebase again

[Wuzzy2]

Rebase done.

[Zughy]

@Wuzzy2 rebase needed again...

[Wuzzy2]

Rebase done. For some reason, I am not allowed to remove labels anymore. :-(

[Desour]

• Tested. Found no other issues.
• Should maybe both, 6 and 7 rotate 90° (or -90°) (as opposed to one 90° and one -90°)? (I guess it doesn't matter though...)
• (devtest: Attached wallmounted node with wallmounted_rotate_vertical = true is missing.)

[Wuzzy2]

Okay, I have done the updates as asked. I also discovered and fixed a bug when placing an attached wallmounted node. This was exactly the node that was missing in DevTest, so adding it was a good idea. :D

Before you ask for another rebase, please first test this and report any remaning issues. Once all issues are resolved, I'll do the rebase. I'm afraid a rebase might be pretty dangerous/tricky this time.

[Zughy]

@Wuzzy2 please rebase docs (see #13438)

EDIT: sorry, I've just noticed your previous comment

[Wuzzy2]

I decided to do the rebase anyway.
Rebase done!

[v-rob]

Unless I misunderstood your idea, this would allow exponentially many combinations: 6 * 2² = 24 instead of the previous 6 + 2 = 8.

Unless I'm mistaken, that's just reinventing facedir (which allows all 24 rotations), but maybe with a different bit representation/client placement. The idea of the PR is to squeeze more possibilities out of the current amount of bits.

[SmallJoker]

The idea of the PR is to squeeze more possibilities out of the current amount of bits.

The PR adds two additional bytes bits to implement two new possible wallmounted states (floor and ceiling). This is not a form of "squeeze out". However, you are right that my proposal is somewhat similar to facedir (or degrotate for that matter). Now that you mention it- I think that wallmounted is redundant if you introduce a new on_place helper function to implement the wall mounting mechanism for facedir nodes.

EDIT: I re-implemented a sign using on_place = minetest.rotate_node. This does currently not disallow stacking "wallmounted" nodes onto each other or ensure that the normal vector points away from the underlying node surface.

assert(default.register_fence, "Needs MTG")

local my_node_box = {
	type = "fixed",
	fixed = {-1/4, -1/2, -1/3, 1/4, -1/4, 1/3},
}

default.register_fence("lab:facedir_wallmounted", {
	description = "FACEDIR WALLMOUNTED",
	texture = "default_fence_wood.png",
	material = "default:stick",
	paramtype2 = "facedir",
	node_box = my_node_box,
	collision_box = my_node_box,
	groups = {choppy = 1, oddly_breakable_by_hand = 1},
	on_place = minetest.rotate_node
})

EDIT: Wrote "bytes" by mistake. My point stands unchanged, though.

[v-rob]

The PR adds two additional bytes to implement two new possible wallmounted states (floor and ceiling)

No, it still only uses 3 bits. 2³ = 8. There are 6 basic wallmounted orientations, and this PR adds 2 more orientations which are 90 degree rotations of the vertical orientations, for a total of 8. This means that colorwallmounted still has 5 bits for palette information.

[Wuzzy2]

v-rob is right and SmallJoker is wrong.
This PR makes use of 2 unused BITS of wallmounted nodes. BITS, not bytes!

The point of this PR is basically to squeeze out more use of the available param2 space while still being compatible.

[v-rob]

This PR makes use of 2 unused BITS of wallmounted nodes. BITS, not bytes!

Ah, but I must be pedantic, since you are also wrong. You use two extra VALUES out of the eight values in the three bits. ;P

I sure wish we had 2 more bytes of param2 space...

[SmallJoker]

@v-rob Actually thank you for being pedantic.

What I misinterpreted: [2 bits: unused] & [6 bits: rotation] (LSB)
What the doc meant: [5 bits: color, optional] & [3 bits: rotation] (LSB)

So there are in fact two states that can further be used without limiting the palette feature ("colorwallmounted"). I see how this PR tries to make the best out of what's free to use. Fine by me.

[Zughy]

@Wuzzy2 in case you missed it #11073 (comment)

[Wuzzy2]

This PR has been rebased.
I've added a core.features flag.

This PR should be ready again now.

[Wuzzy2]

@Desour: Oops! Fixed.

[Wuzzy2]

I just noticed this PR has two approvals. However, due to heavy rebasing, request changes and chaos, I strongly recommend to do some final testing before merging, just in case.

My own tests look fine.

[srifqi]

This PR still works.

[Desour]

Indeed still works.