[3.x] Change ObjectID to a class instead of typedef

[lawnjelly]

Fixes #107616

3.x backport of cf8c679

Introduction

Investigation shows that 3.x is riddled with bugs caused by 32 bit intermediates storing ObjectIDs, which are actually 64 bit. When the 32 bit values are used to get_instances() the results will be haywire once we reach the 32 bit limit.

Additionally I've done some preliminary work on a far improved ObjectDB, with far safer and faster behaviour, but this depends on using true 64 bit ObjectIDs, hence the push to finally sort out this problem in 3.x.

Godot 3.x ObjectIDs currently

In Godot 3.x before this PR, ObjectID is merely a typedef to uint64_t. This made it incredibly difficult to compile time check for errors, because C will happily promote 32 bit values to 64 bit. This has allowed numerous bugs to creep into the codebase. The solution adopted by Godot 4.x, and backported in this PR, is to make ObjectID a fully fledged class, where promotion can be explicitly prevented.

Although I've backported 4.x PR in spirit, there are some differences in how I'm handling gdnative that are more appropriate for 3.x, in order to minimize changes.

is_valid() and is_null()

4.x offers both these functions for determining validity. However in the core meeting last week (18th June 25) a number of core maintainers expressed misgivings that two functions had been exposed in 4.x, and believed it would have been better to expose a single version (is_valid()), and negate it. Although it is too late to change in 4.x, I have modified this PR to just offer the is_valid() function based on this feedback.

Like the 4.x commit by @reduz, this PR touches a number of areas, some of which I was previously unfamiliar with, especially:

GDNative

Although it is likely very few people (if any) are using GDNative at this point with 3.7+, I wanted to remain compatible if possible. GDNative was a mess, the godot_instance_from_id() function currently takes a godot_int which is 32 bit.

On advice from @vnen, I have deprecated the old function, and added a new one as "core 1.5" API.

While the 4.x PR changes godot_int to int64_t, that would have been majorly compat breaking for 3.x, so instead I opted to add a new function:

godot_get_instance(uint64_t).

So users can call get_instance_id(), store a uint64_t (for the ObjectID) and use the new function. The old function now displays a warning message, and always returns NULL, as it can no longer be guaranteed to work with > 32 bit ObjectIDs.

This does mean any users storing ObjectIDs and using this function with gdnative will need to make some small modifications to their gdnative code for 3.7.

This will also entail a small PR for godot-cpp, adding this third line to godot-cpp/godot-headers/gdnative.h:

// equivalent of GDScript's instance_from_id
godot_object GDAPI *godot_instance_from_id(godot_int p_instance_id);
godot_object GDAPI *godot_get_instance(uint64_t p_instance_id);

https://chat.godotengine.org/channel/core?msg=Qs2HDBKmX4Ywpt5k5

vnen
GDNative does have a system for compatibility. Essentially you can add an "extension" with new functions, and only libraries aware of it will load those. It's a bit convoluted tho

lawnjelly
Ah that looks straightforward enough. But can we alter an existing function though?
The issue is the existing function for get_instance() is duff because it takes an int and it needs to be a uint64_t.
unread messages

vnen
I don't remember exactly, but I think you can add a function with the same name. The name in the JSON is what counts IIRC
If you change the existing function things are going to crash. Although a bogus object id will probably also lead to crash, so I don't know

lawnjelly
Maybe we can create a new function name for the 64 bit, than output a big message when they try to use the old function.

vnen
Yeah, that sounds good

lawnjelly
To be honest the amount of users this will affect is going to be miniscule at this point I suspect.
The cross section of still using 3.x, and using GDNative...
and it's quite an easy fix on their side for this if they are compiling GDNative.

vnen
Likely zero. Maybe the bindings will have issues, but even those are not maintained I think

Mono

This had a 64 bit value available so should work as is.

JNI

jlong seems to be an int64_t, so this was fairly straightforward, and should work as is.

Notes

• I've not bound PROPERTY_HINT_INT_IS_OBJECTID because it is deprecated in 4.x, and looks used internally only.
• Other than the above exceptional areas, this should just work. ™️