-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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 uuidv4_text function #42408
Add uuidv4_text function #42408
Conversation
c1f347f
to
ab4520b
Compare
Pull requests should be opened against the |
Sorry about that. I should have opened the PR for the |
For reference, this is what I've been talking about in: #39871 (comment). |
Co-authored-by: Hugo Locurcio <[email protected]>
bd30dce
to
fba988c
Compare
Is there any good reason to add this? It could be done with a script. |
I'm using for:
Why it is done via C++ and not GDScript, because the C++ function outperforms GDScript addon (10x faster). |
Sure it's faster, but it doesn't look like performance critical code and you could do it with GDNative to get better performance. |
If he's using it for a database, then it's absolutely performance critical. |
@Zireael07 In a database, you usually have to perform this UUIDv4 conversion only every time you insert an element. Most databases don't even get close to 100 inserts a second 🙂 Also, in many cases, your database can generate an UUID or UUID-like value automatically when inserting a row (see MySQL's |
Getting a unique identifier accross the world is useful for a game. Specially if you make a multiplayer game. |
That's what Git branches are for 🙂 Such an add-on doesn't require a great amount of maintenance. It doesn't interact with any of the editor. |
Why add that work when you can make it built-in and would require no work at all? |
When we add something to core, it shifts the maintenance burden from the user to core developers. Once we add it, we essentially commit to supporting it forever. However, we only have a finite amount of manpower. I recommend you open a proposal to gather community support on this feature. |
Regarding
We know GDScript is slow, so the performance argument raised before is not valid. Since there are others PR that have been merged without having any proposals and have a much larger impact than this one, so I don't understand why you are recommanding to open a godot proposal. Also, this PR has more like than most of the PRs I'm quoting.
To generate one ID (average):
Now let's say Im inserting a complex object, so Ill have to generate 10 ids with one action, I wasted ~1ms because of GDScript. |
Nothing is eternal. #42113 is a clear example of this.
From my observations, we keep gaining more contributors, not less, I'd safely extrapolate that at least 1% of those contributors will become one of those developers who can maintain the engine.
There are generally many other smaller features like this which are difficult to keep feature branches for. One has to either use a patching system and/or maintain a custom version of the engine to have persistent, reproducible core functionality across game projects (which also require performance), but that's only applicable for in-house solutions... Also, I've seen a tendency with Godot development that even the smallest features attract controversial discussions. I don't understand this. I believe this PR is general-purpose enough to cater most needs. By "most" I mean is that, at some point of development, features like this are essential to have in any big project with enough of playerbase, which may not be useful for beginner developers just yet. Talking about the "adoption" by game development companies across the world as desired by some core developers, alas the current development approach and evaluation of features don't seem to facilitate this. If Godot Engine caters mostly to beginner use cases, then this should be written on the Godot Engine homepage. But do note that the current wording doesn't reflect this:
|
This should use crypto rand instead of math rand, and the UUID v4 standard does not include timestamps (those are used by other UUID versions) |
Why you add function to I think that you need to create var id := UUID.create_v4();
var id_text := id.to_string(); You can create it as a module and request addition to default modules. |
RFC for UUID v4:
Later in this doc
Later in this doc
|
Since the String class has hash, md5 and sha functions I though it would be a good candidate for uuid: |
Since this PR is dead and it is not going to be merged, Im closing it. |
A the function
uuidv4_text()
to GDScript, and to theString
classIt's been tested on Windows, Mac and Linux
Godot Proposal