# Clamp(vector) doesn't do what I expect it to

4.2.1

### Question

Does clamping a vector mean something other than clamping the individual components or is the following a bug?

### Scenario

I wrote the below function to translate from pixels to a ‘map’ and it contained the code:

``````clamp( map_coord - Vector2i.ONE
, Vector2i.ZERO
, Vector2i( MAP_X_DIM, MAP_Y_DIM )
)
``````

Note that `map_coord` is `Vector2i`
If `map_coord` is `( 0, n * MAP_Y_DIM )`, then the code returns `( 0, n )`.
If `map_coord` is `( n * MAP_X_DIM, 0 )`, then the code returns `( n, -1 )`.

That second result is surprising (to me).

I have re-written the code to this:

``````Vector2i( clampi( map_coord.x - 1, 0, MAP_X_DIM )
, clampi( map_coord.y - 1, 0, MAP_Y_DIM )
)
``````

Which returns `(0, n)` and `(n,0)` as expected and is how I would imagine `clamp(vector...)` would work.

Can you provide the code.

``````const MAP_X_DIM        : int = 28
const MAP_Y_DIM        : int = 15
const MAP_PX_PER_UNIT  : int = 64

# This version clamps a vector
func _map_coord_from_px_CV( px_coord : Vector2 ) -> Vector2i :

var map_coord : Vector2i = ( px_coord / MAP_PX_PER_UNIT ).round()

return clamp( map_coord - Vector2i.ONE, Vector2i.ZERO, Vector2i( MAP_X_DIM, MAP_Y_DIM ))

# This version clamps the components and make a vector
func _map_coord_from_px_VC( px_coord : Vector2 ) -> Vector2i :

var map_coord : Vector2i = ( px_coord / MAP_PX_PER_UNIT ).round()

return Vector2i( clampi( map_coord.x - 1, 0, MAP_X_DIM ), clampi( map_coord.y - 1, 0, MAP_Y_DIM ))

# This is the reciprocal code used in the test cases in `_ready`, below
func _map_coord_to_px( map_coord : Vector2i ) -> Vector2 :

var px_coord : Vector2 = ( map_coord + Vector2i.ONE ) * MAP_PX_PER_UNIT

return px_coord

func _ready() :

var test_vectors : Array[Vector2] = [ Vector2( 0, 1000 )
, Vector2( 1665, 0 )
, _map_coord_to_px( Vector2i.ZERO )
, _map_coord_to_px( Vector2i.ONE )
, _map_coord_to_px( Vector2i( 27, 14 ))
, _map_coord_to_px( Vector2i( 28, 15 ))
]

for px_coord in test_vectors:
var map_coord = _map_coord_from_px_CV( px_coord )
print( "CV px: ", px_coord, "; map: ", map_coord )

for px_coord in test_vectors:
var map_coord = _map_coord_from_px_VC( px_coord )
print( "VC px: ", px_coord, "; map: ", map_coord )
``````

Attach that to a random node and run your project. I receive the following output:

``````CV px: (0, 1000); map: (0, 0)
CV px: (1665, 0); map: (25, -1)
CV px: (64, 64); map: (0, 0)
CV px: (128, 128); map: (1, 1)
CV px: (1792, 960); map: (27, 14)
CV px: (1856, 1024); map: (28, 15)
VC px: (0, 1000); map: (0, 15)
VC px: (1665, 0); map: (25, 0)
VC px: (64, 64); map: (0, 0)
VC px: (128, 128); map: (1, 1)
VC px: (1792, 960); map: (27, 14)
VC px: (1856, 1024); map: (28, 15)
``````

Note the -1 on line 2

I just noticed line 1 is wrong, also…

Yep definitely broken. I can’t see what us the problem but for better safty you can use Vector2i.clamp() function. Btw why is subtracting Vector2i.ONE is necessary?

1 Like

As far as I understand clamp() compares the passed arguments with the `>` operator, which does this for Vectors:
Compares two Vector2 vectors by first checking if the X value of the left vector is greater than the X value of the right vector. If the X values are exactly equal, then it repeats this check with the Y values of the two vectors. This operator is useful for sorting vectors.
So no, it doesn’t clamp individual elements.

But using clamp on the vector2 itself like: `my_vector2.clamp(min, max)` does clamp the elements as you’d expect.

1 Like

Regarding the Vector2i.ONE, this is because the play area has a border that is approximately 64 pixels wide.

That’s still a bug, though, right?

IMHO, the library should either not support vectors or support them properly.

Well, clamp() is implemented as a function template, as it is right now, it can only handle vectors the same way it handles any other variable (which is figuring out which one is greater using `>`, as I’ve said):

``````template <typename T, typename T2, typename T3>
constexpr auto CLAMP(const T m_a, const T2 m_min, const T3 m_max) {
return m_a < m_min ? m_min : (m_a > m_max ? m_max : m_a);
}
``````

So it’s not really a bug, just a limitation of using a single template I guess. I had the same issue as you, so I agree it’s not ideal…

I can see two solutions to this, one in the compiler of gdscript and one in C++ itself. As I have no idea how gdscript is implemented, I don’t know which would work best.

In gdscript compiler, there could be code somewhere that says:

``````if m_a.has_method( "clamp" ):
emit( "m_a.clamp( m_min, m_max" )
else:
emit( "clamp( m_a, m_min, m_max" )
``````

You can do something similar in C++ directly with template specialisation but it’s been a LONG TIME since I’ve had to read, let alone write, anything so gnarly.

1 Like

Well I just like to dig around in the code, but where / how you’d fix it is beyond my expertise You’d have to raise an issue on github I guess if you’d like it to change.

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.