Mark multiply and divide with two vectors as deprecated (Schur product and quotient)

[Desour]

• vector.add with a scalar in the second argument and mul with two vectors (and sub and div) is very unnatural and useless.
  Who needs the Schur product or quotient or a value added or subbed to/from all components?
  I do not think that I have seen them being used in code anywhere.
• Therefor these functions are marked as deprecated.

To do

This PR is a Ready for Review.

How to test

Look at the documentation.

[paramat]

Who needs the Schur product or quotient or a value added or subbed to/from all components?

These have always seemed odd and near-useless to me too.

[sorcerykid]

I disagree. Adding or subtracting with a scalar as the second argument can be useful in applications where a radius is provided, and one simply wishes to compute the minimum and maximum positions from a central position.

Here's a code snippet from my Protector Redux mod:

local function lock_area( meta, source_pos, content_ids )
        local pos1 = vector.subtract( source_pos, config.protector_radius )
        local pos2 = vector.add( source_pos, config.protector_radius )

        local voxel_manip = minetest.get_voxel_manip( )
        local min_pos, max_pos = voxel_manip:read_from_map( pos1, pos2 )
        local map_buffer = voxel_manip:get_data( )
        local voxel_area = VoxelArea:new( { MinEdge = min_pos, MaxEdge = max_pos } )

It's kind of nice not having to create a temporary vector just for two statements, or to code it verbosely as

        local pos1 = vector.subtract( source_pos, {
                x = config.protector_radius,
                y = config.protector_radius,
                z = config.protector_radius )
        local pos2 = vector.add( source_pos, {
                x = config.protector_radius,
                y = config.protector_radius,
                z = config.protector_radius )

In my view, I think having shorthands even if used infrequently can make code easier to understand and maintain.

[pyrollo]

When I first saw add and sub with scalar, I thought this was useless. Then I saw how it was used in some code. Now my opinion is the same as Sorcerykid.

These two forms should not be deprecated as they are quite useful.

For mul and div forms, it's important to figure out if it's really useless.

[Desour]

Ah, I see, the weird add and sub are used for making cubes.
(I would probably not use it in such situations because giving three values is more clear and lies the emphasis on it being a cube.)

[paramat]

Thanks for the good input, i did not think of radii.
I agree, we should only possibly deprecate the Schur product/quotient.

[Desour]

Another problem with the Schur product is that it would not be associative with the matrix product (if we will have it at some point):

((m * v) * w).x = (m[1][1] * v.x + m[1][2] * v.y + m[1][3] * v.z) * w.x
(m * (v * w)).x = m[1][1] * v.x * w.x + m[1][2] * v.y * w.y + m[1][3] * v.z * w.z
=> (m * v) * w != m * (v * w)

I'll remove the change to add and sub.

[pyrollo]

According to wikipedia Schur product "appears in lossy compression algorithms such as JPEG". Not likely to have a JPEG compression algorithm in Lua :)

[rubenwardy]

I reckon that + shouldn't support schur product at all, and there should be method for different types of product (dot, schur, ..)

[Desour]

I reckon that + shouldn't support schur product at all, and there should be method for different types of product (dot, schur, ..)

You mean *.
I agree.
Should I add a vector.schur in this PR for replacement (just in case anyone needs it).

[paramat]

Should I add a vector.schur in this PR for replacement (just in case anyone needs it).

Nah ... i doubt it is much needed, we could always add one later if users complain.

👍

[FreeLikeGNU]

I just noticed the commit and I use vector.add and vector.multiply for my witches spell effects:

local new_target_pos = vector.add(target_pos, vector.multiply(vector.direction(caster_pos, target_pos),strength))

new_target_pos.y = target_pos.y + height

target:set_pos(new_target_pos)

witches.magic.effect_line01(caster_pos,new_target_pos,50)

I've been evacuated from my home because of a forest fire for the past week so I apologize for not checking out this PR sooner.

[sfan5]

vector.add is not affected by this.
And assuming strength in your code is a number, your use vector.multiply is not affected by this either.

[FreeLikeGNU]

Ah OK, I misunderstood. My apologies.

[appgurueu]

Big mistake. I use this to scale vectors (to reverse the effects of visual scale when attaching, for instance).

[Desour]

There's always the possibility to change the docs again to make the schur product non-deprecated.
It was probably indeed a bad idea to make a PR just to mark something deprecated (considering it won't be removed in the foreseeable future anyways). (=> nobody is really more happy, and some don't like the change)