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

Add a built-in UUIDv4 function #1654

Closed
xsellier opened this issue Oct 13, 2020 · 17 comments
Closed

Add a built-in UUIDv4 function #1654

xsellier opened this issue Oct 13, 2020 · 17 comments

Comments

@xsellier
Copy link

xsellier commented Oct 13, 2020

Describe the project you are working on:
City Game Studio

Describe the problem or limitation you are having in your project:
I don't have any problem, but a uuidv4 function would be great since Im using it extensively.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Having a uuidv4 builtin function within the engine itself improved the performances and removed most of the stuttering.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
godotengine/godot#42408

If this enhancement will not be used often, can it be worked around with a few lines of script?:
https://github.com/binogure-studio/godot-uuid

Is there a reason why this should be core and not an add-on in the asset library?:
I have worked on many games before (solo and with a small team of 50 people), and all of them needed a uuidv4 generator. So, if it is widely used it should be part of the engine itself.

@Calinou Calinou changed the title Add a uuidv4 builting function Add a built-in UUIDv4 function Oct 13, 2020
@dominiks
Copy link

dominiks commented Oct 13, 2020

I too have started to use UUIDs in my projects in places where I'll synchronize objects with multiple clients and think there is a need for UUIDs in game development and that they should be added to Godot.

So which implementation are you proposing? The PR has a UUID v1 (somewhat) based on timestamp while your example script implements v4 according to RFC-4122. I think v4 should be used.

@xsellier
Copy link
Author

It is the UUIDv4 implementation Im talking about.

@jonbonazza
Copy link

I agree uuidv4 should be implemented. I'd probably just call the function uuid though as v4 is simply an implementation detail and most users won't care or even know the difference. There will likely only be one version implemented anyway.

@realkotob
Copy link

I use uuid4 in godot in a single-player game to generate identifiers for player-created levels so they can be shared without needing a centralized id-giver.

👍 for this

@dominiks
Copy link

It is the UUIDv4 implementation Im talking about.

Can you edit the PR then to provide an implementation analogous to the GDScript?

@LikeLakers2
Copy link

LikeLakers2 commented Oct 14, 2020

I'd probably just call the function uuid though as v4 is simply an implementation detail and most users won't care or even know the difference.

I agree, it would be simpler to call it uuid. However, knowing that the function generates version 4 UUIDs may be important to some users (even though that number may be pretty small) -- so that should be mentioned, perhaps in the documentation. Maybe include a little note in the function's description saying "This generates version 4 UUIDs."

@xsellier
Copy link
Author

It is the UUIDv4 implementation Im talking about.

Can you edit the PR then to provide an implementation analogous to the GDScript?

It is done the same way, without collision.
And it is based on the timestamp plus a rand
https://github.com/godotengine/godot/pull/42408/files#diff-a820694568b88ae346f4144afa4427c65dafd323b508fd05ad8f324e140f9d7aR1009

@dominiks
Copy link

dominiks commented Oct 14, 2020

Yes, but timestamp plus a rand is not v4. See the RFC for a reference:

4.4. Algorithms for Creating a UUID from Truly Random or
Pseudo-Random Numbers

The version 4 UUID is meant for generating UUIDs from truly-random or
pseudo-random numbers.

The algorithm is as follows:
o Set the two most significant bits (bits 6 and 7) of the clock_seq_hi_and_reserved to zero and one, respectively.
o Set the four most significant bits (bits 12 through 15) of the time_hi_and_version field to the 4-bit version number from Section 4.1.3.
o Set all the other bits to randomly (or pseudo-randomly) chosen values.

@xsellier
Copy link
Author

So you want something like that?

	uint8_t bytes[16] = {
		// time low
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),

		// time mid
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),

		// time high
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),

		// clock seq hi
		(uint8_t)((Math::rand() & 0x0f) | 0x40),

		// clock seq low
		(uint8_t)(Math::rand() & 0xff),

		// node
		(uint8_t)((Math::rand() & 0x3f) | 0x80),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff),
		(uint8_t)(Math::rand() & 0xff)
	};

@dominiks
Copy link

That looks like a uuidV4 implementation. If you want sequential UUIDs based on timestamp you can add them as a V1 implementation that includes the OS.get_unique_id() in some form for the UUID-node id or just randomize it.

@Yukitty
Copy link

Yukitty commented Oct 20, 2020

Yukitty@godot-addon-crypto_uuid_v4
I like the concept of immutable UUID objects (like a subset of String that explicitly knows it's supposed to be a unique valid UUID and stores its data in an efficient PackedByteArray?), but failed to figure out how GDScript can adequately provide that kind of behavior.

So instead, here's an implementation that provides UUID refrence objects with an is_equal comparison function and lazy optional string conversion.
oh, and I used cryptographic RNG where available to prevent collisions, including special consideration for web builds.

@dominiks
Copy link

dominiks commented Oct 20, 2020

Ok now I feel like someone is watching me work. I have stumbled upon Crypto.generate_random_bytes just seconds ago and thought about using that for my UUID generation.

Really like your implementation and I'll probably just use your addon.

Regarding the immutability aspect I think you can prevent outside access to _data and _string by giving them a setter that does not change the actual value (or throw a warning or sth). As long as you don't use self when writing these fields you will bypass the setter method. That's not the most elegant way of doing that but might work.

@Yukitty
Copy link

Yukitty commented Oct 20, 2020

Forceful no-op setters are not the GDScript way. 😜
Just putting the underscore there and calling it a private variable or function is enough for other developers to understand.

My actual issue is, wanting to make sure that every UUID.new(from) (or nominally UUID(from) with an engine plugin implementation) with the same input data from always returns the same exact object, like when loading Resources from the same path. (Eg. for constructing UUIDs as part of objects during deserialization, and references to other objects or cached assets via matching UUIDs.)

That would make simple object comparisons super easy, since you could just do uuid_a == uuid_b instead of calling uuid_a.is_equal(uuid_b) and comparing the actual data at all, you see.

@Xrayez
Copy link
Contributor

Xrayez commented Oct 29, 2020

Forceful no-op setters are not the GDScript way. 😜

GDScript has a known bug for this as well godotengine/godot#41319, so I try to avoid using getter without a setter in GDScript.

@Xrayez
Copy link
Contributor

Xrayez commented Oct 29, 2020

Random implementations could be in theory added in Random singleton, like Random.uuid(), as proposed in #1741.

@xsellier
Copy link
Author

xsellier commented Dec 5, 2020

I don't want to flood the godot-proposals repository. And since this proposal doesn't seem to get implemented anytime soon, I'd rather close it to give more visibility to other proposals who might be implemented.

@filipworksdev
Copy link

filipworksdev commented Mar 30, 2022

If anyone is interested I have a custom build where I use a global singleton Rand class (similar to what Xrayez proposes) with a lot more random functions including uuidv4 called like this Rand.uuid4() see here for implementation goblinengine/goblin@38690e0. Posting this in case someone needs a C++ uuid4 implementation for Godot.

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

No branches or pull requests

9 participants