Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bsearch_custom seems to swap arguments depending on "before" argument. #89276

Closed
donn-xx opened this issue Mar 8, 2024 · 1 comment · Fixed by #89280 or #69451
Closed

bsearch_custom seems to swap arguments depending on "before" argument. #89276

donn-xx opened this issue Mar 8, 2024 · 1 comment · Fixed by #89280 or #69451

Comments

@donn-xx
Copy link

donn-xx commented Mar 8, 2024

Tested versions

v4.3.dev.custom_build [9f4499c]

System information

Godot v4.3.dev (9f4499c) - Ubuntu 22.04.4 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 2060 (nvidia; 535.161.07) - Intel(R) Core(TM) i5-9400F CPU @ 2.90GHz (6 Threads)

Issue description

The code is below. This is the output, with the error included.
When before is true, arg1 is the sub-array. When false, arg1 switches to become the float. I'm not sure if this is intended or not, but it feels like a bug.

When before is true
arg1:[2, 2]
arg2:1.1
arg1:[3, 2.5]
arg2:1.1
4

When before is false
arg1:1.1
arg2:[2, 2]

Invalid access to property or key '1' on a base object of type 'float'.

PS - Can anyone explain what before is supposed to do anyway? The docs need an example.

Steps to reproduce

extends Node2D

func _ready() -> void:
	test()

func test():
	# "The custom method receives two arguments (an element from the array and the value searched for)"
	var cmp = func(arg1,arg2): print("arg1:",arg1); print("arg2:", arg2);return arg1[1]>=arg2
	var X:=[ [0, 1.0], [1, 1.5], [2, 2.0], [3, 2.5] ]
	print("When before is true")
	var index = X.bsearch_custom( 1.1, cmp, true )
	print(index)
	print()
	print("When before is false")
	# We get an error:
	#  Invalid access to property or key '1' on a base object of type 'float'.
	# Because the arguments to `cmp` have swapped!
	index = X.bsearch_custom( 1.1, cmp, false )
	print(index)

Minimal reproduction project (MRP)

https://gitlab.com/dbat/array_bsearch_issue

@AThousandShips
Copy link
Member

AThousandShips commented Mar 8, 2024

What the argument does is given in the documentation:

Optionally, a before specifier can be passed. If false, the returned index comes after all existing entries of the value in the array.

It's essentially changing between lower/upper bound search, the code is this:

if (p_before) {
while (lo < hi) {
const int64_t mid = (lo + hi) / 2;
if (compare(p_array[mid], p_value)) {
lo = mid + 1;
} else {
hi = mid;
}
}
} else {
while (lo < hi) {
const int64_t mid = (lo + hi) / 2;
if (compare(p_value, p_array[mid])) {
hi = mid;
} else {
lo = mid + 1;
}
}
}
return lo;

You can't assume the order of the comparison, this isn't an error, the comparison function is supposed to take both types, though the fact it can take both orders might be made clear though it never says it is in the order you expected:

The custom method receives two arguments (an element from the array and the value searched for) and must return true if the first argument is less than the second, and return false otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants