Fix automatic rotate for attached entities

[appgurueu]

Trivial fix to make automatic rotation work for attached entities.

How to test

minetest.register_entity("testentities:spinner", {
	initial_properties = {
		visual = "cube",
		textures = {"a", "a", "a", "a", "a", "a"},
		automatic_rotate = math.pi,
	},
	on_activate = function(self)
		self.object:set_armor_groups({punch_operable = 1})
	end,
	on_punch = function(self, puncher)
		self.object:set_attach(puncher)
	end
})

then /spawnentity testentities:spinner and punch it to attach

[sfan5]

This is so trivial it's suspicious. Can you try to find out why it was disabled for attached entities?

[appgurueu]

1. b22168d initially introduced this feature for dropped items; attachments didn't exist back then
2. 52fcb0b#diff-4e878b8f5e372f965d18fca8ef0492cacbd881af375263277e55ef2e6c138728L1050 added this check but no reason seems to be given; at that time the automatic rotation was implemented over set_yaw however which means it couldn't possibly work for attachments
3. Make automatic_rotate relative, allow setting rotation #8468 changed the implementation to use the scene node, which means removing the check now works fine (at least from my brief testing of this PR); the PR author @pgimeno already anticipated that, but apparently planned a separate PR?

What is not covered by this patch is rotation of attachments. This means that e.g. a propeller attached to a plane is not possible without adding a new property. Current plans are to submit a separate PR for that.

With all this in mind it's worth noting that at the time this check was added it was reasonable because setting the yaw would've taken no effect either way; I just wonder why @pgimeno's PR didn't remove it and would like to hear his input on this.

That said this PR seems to work fine and I believe extensive testing of this is worth more than trying to dig up the past.

[erlehmann]

removing the check now works fine (at least from my brief testing of this PR)

What exactly did you test? Given that the possible attachment chains can be as hilarious “a player with arrows in them sits on a mob that is in a boat that is in a minecart”, I am as suspicious of this as @sfan5 is.

[appgurueu]

removing the check now works fine (at least from my brief testing of this PR)

What exactly did you test? Given that the possible attachment chains can be as hilarious “a player with arrows in them sits on a mob that is in a boat that is in a minecart”, I am as suspicious of this as @sfan5 is.

Then please test it.

[sfan5]

Works as advertised

[appgurueu]

@corarona @erlehmann turns out MCL2 relies on this being broken; wielditems have started spinning since they have automatic rotate set...

See the following video by @rollerozxa: https://media.discordapp.net/attachments/369123125021114369/999773166656290846/2022-07-21_22-21-42.mp4

The fix is trivial: Just remove the line https://git.minetest.land/MineClone2/MineClone2/src/commit/69d1c26a155af0d7315ac29cd37060928b9cde24/mods/PLAYER/mcl_wieldview/init.lua#L87

[erlehmann]

@appgurueu wait, why is it set there though?

This is so trivial it's suspicious.

Wise words.

[erlehmann]

@EliasFleckenstein03 according to git blame you added the automatic_rotate = 1.5 there. Why did you do it?

@rubenwardy can we get a CDB search for automatic_rotate to determine what else this might have affected?

[rubenwardy]

https://content.minetest.net/zipgrep/d0da7b4d-67e6-405e-8f50-3f0c6491da43/

[appgurueu]

@appgurueu wait, why is it set there though?

This is so trivial it's suspicious.

What's suspicious is that modders have been cargo-culting arbitrary automatic_rotate values in their object properties.

[LizzyFleckenstein03]

@EliasFleckenstein03 according to git blame you added the automatic_rotate = 1.5 there. Why did you do it?

When I rewrote the wieldview mod I copied some of the old code. No idea what automatic_rotate does tbh.

[erlehmann]

What's suspicious is that modders have been cargo-culting arbitrary automatic_rotate values in their object properties.

For future reference: Full recognition before processing (i.e. following LANGSEC principles) would have prevented this.