rhotator does not conform to screwdriver (on_rotate) API

[fluxionary]

see https://github.com/minetest/minetest_game/blob/1e237b8d18da799593ad3a8253cdc5d8a7d772a9/game_api.txt#L594-L610

the main issue is that the rhotator clobbers the semantics of the mode parameter:

https://github.com/minetest/minetest_game/blob/1e237b8d18da799593ad3a8253cdc5d8a7d772a9/mods/screwdriver/init.lua#L9-L10

vs.

https://github.com/entuland/rhotator/blob/e2a928349e5896b438c0ab779ef3a632884d90b8/init.lua#L18-L19

it also ignores the expected range of param2 values that would go along with these modes, which can result in nodes being rotated into invalid positions. the best solution i can think of, is to change the values to something like

local PRIMARY_BTN = 101
local SECONDARY_BTN = 102

[entuland]

Sorry, unfortunately I am not able to do any investigation on this problem - not even fully sure I understand where the problem lies as I'm pretty much using "primary" and "secondary" as variable names for 1 and 2 just for my own "code cosmetics" choice in the context of how rhotator works - rhotator normally "does its thing" if on_rotate() isn't defined, but when it is defined and it gets called, on_rotate() expects exactly those two values (1 or 2) to mean rotate face or rotate axis, so I'm not sure what's the effect and rationale of changing the value of my constants to 101 and 102.

If you are able to provide a patch along with specific examples of nodes which are being rotated incorrectly with the current code and verify that they do get rotated correctly with your patch I'll try to find the time to test and possibly merge such changes.

[fluxionary]

I'm pretty much using "primary" and "secondary" as variable names for 1 and 2 just for my own "code cosmetics" choice in the context of how rhotator works - rhotator normally "does its thing" if on_rotate() isn't defined, but when it is defined and it gets called, on_rotate() expects exactly those two values (1 or 2) to mean rotate face or rotate axis, so I'm not sure what's the effect and rationale of changing the value of my constants to 101 and 102.

sorry it's taken me a while to respond. the issue is that the param2 values supplied to on_rotate do not correspond to "rotate face" or "rotate axis". this can result in nodes being rotated into invalid positions, e.g. this piece of insulated mesecons:

[fluxionary]

i picked "101" and "102" arbitrarily; the main point is that they should not be "1" and "2" unless the same range of param2 values is guaranteed.

[entuland]

Sorry, as I mentioned I don't have the time to investigate the issue.

Recalling how I set things up I have a feeling that my code is doing the right thing by passing only 1 or 2 as requested by the API, and setting param2 according to how I would have set it _if onrotate() wasn't defined at all, again as requested by the API.

To the best of my knowledge any param2 I'm passing should be valid for the paramtype2 of that node, my best guess is that that node you refer to has a wrong paramtype2 that misleads any screwdriver-like mod into thinking that such param2 values would be acceptable.

I'd think also that if the values were inacceptable, it would be up to that on_rotate() handler to reject the rotation request (notice that my code does not touch the node if there's an on_rotate() handler set at all, when such a handler is set at all, I'm passing the values I think should be applied according to the paramtype2 declared by that node, and I'm delegating any action to the on_rotate() handler itself, which is of course not part of my mod).

If you still believe that my code is doing the wrong thing please provide the correct code along with the details of which param2 values are being passed to on_rotate() and why such values are wrong for the specific paramtype2 that the node itself declares, so that I can evaluate your code for merging.