Godot: AStar throws error for ids greater than 2^31 - 1

[wingedadventurer]

Godot version:
3.2.3.stable

Issue description:
Max integer size in Godot is 2^63 - 1, as stated here. However, if you provide an id to AStar that is greater than 2^31 - 1, it will throw an error.

I took a peek in AStar source code:

void AStar::add_point(int p_id, const Vector3 &p_pos, real_t p_weight_scale) {
    ERR_FAIL_COND(p_id < 0);
    // ...

I believe the issue is that Godot's int is "Signed 64-bit integer type" while p_id argument that's of type int is something lower, probably signed 32-bit.

I discovered the issue when generating random ids for a waypoint system, where upper limit is Godot's int max. For now i'll just reduce the upper limit to be 2^31 - 1.

Steps to reproduce:
Add points to AStar with ids greater than 2^31 - 1.

[Bhuvan-Vemula]

Interesting ,
but In Cpp int takes value from -2 ^ 31 to (2^31)-1. so anything greater than ((2^31) - 1) + x will be handled as (-(2^31) + x) which causes error in the next line.

i bet u known this Stuff but Is it really necessary to make the whole implementation in long long int To support it the GDScript Standard. ?

If yes, I'm In.

[Calinou]

Shoudln't replacing int with int64_t do the trick?

[Bhuvan-Vemula]

Yep.

[Zylann]

FYI, I think there are a number of places (basically everywhere) in which int is used anyways, so I wouldnt be surprised if an issue like this happens elsewhere. The question would then be if we really want to support 64-bit integers in all these classes.
(64-bit uints in AStar nodes seems a bit extreme, are you encoding flags or Steam IDs in it or something?)

[slapin]

Can't consider this a bug, maybe a documentation issue. You can always have multiple AStar nodes as for this large number of nodes AStar will take seconds or even minutes to complete. No much point for 64-bit ids.

[akien-mga]

This needs design discussion beyond this specific AStar case, as we indeed use int throughout the codebase and many methods have similar restrictions. If this is by design / for performance, we need a way to make this appear clearly in the documentation, and the simplified Variant::INT and Variant::FLOAT doesn't really allow it for now.