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

OS.get_unique_id() returns empty string on Web target #82439

Closed
kxn opened this issue Sep 27, 2023 · 3 comments · Fixed by #82441
Closed

OS.get_unique_id() returns empty string on Web target #82439

kxn opened this issue Sep 27, 2023 · 3 comments · Fixed by #82441

Comments

@kxn
Copy link

kxn commented Sep 27, 2023

Godot version

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Compatibility) - AMD Radeon RX 5700 XT (Advanced Micro Devices, Inc.; 31.0.21001.45002) - Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz (16 Threads)

System information

Godot v4.1.1.stable - Windows 10.0.22621 - Vulkan (Compatibility) - AMD Radeon RX 5700 XT (Advanced Micro Devices, Inc.; 31.0.21001.45002) - Intel(R) Core(TM) i7-10700K CPU @ 3.80GHz (16 Threads)

Issue description

OS.get_unique_id() returns empty string

there is an error in developer console as well

1695812788144

Steps to reproduce

func _ready():
print(OS.get_unique_id())

Minimal reproduction project

not needed

@kxn kxn changed the title OS.get_uniqueid() returns empty string on Web target OS.get_unique_id() returns empty string on Web target Sep 27, 2023
@akien-mga akien-mga added this to the 4.2 milestone Sep 27, 2023
@akien-mga
Copy link
Member

Indeed OS.get_unique_id() is not implemented in platform/web/os_web.cpp, so it falls back to the default implementation in core/os/os.cpp which is just:

String OS::get_unique_id() const {
	ERR_FAIL_V("");
}

I vaguely remember that getting a unique ID on Web may not be possible by design, as it's considered a security issue to let websites identify and thus track users like this. But there should be a proper override that makes this clear, possibly by printing a more explicit error, and maybe a non-unique fallback value.

@AThousandShips
Copy link
Member

This is also documented

@akien-mga akien-mga added enhancement and removed bug labels Sep 27, 2023
akien-mga added a commit to akien-mga/godot that referenced this issue Sep 27, 2023
Remove the base error message in `OS`, we no longer really error out this
way for not implemented methods. Instead, each platform should override them
to provide the context they want.

Fixes godotengine#82439.
@kxn
Copy link
Author

kxn commented Sep 28, 2023

Indeed OS.get_unique_id() is not implemented in platform/web/os_web.cpp, so it falls back to the default implementation in core/os/os.cpp which is just:

String OS::get_unique_id() const {
	ERR_FAIL_V("");
}

I vaguely remember that getting a unique ID on Web may not be possible by design, as it's considered a security issue to let websites identify and thus track users like this. But there should be a proper override that makes this clear, possibly by printing a more explicit error, and maybe a non-unique fallback value.

Agree that a non-unique fallback value would be better, e.g a random uuid

@kxn kxn closed this as completed Sep 28, 2023
mandryskowski pushed a commit to mandryskowski/godot that referenced this issue Oct 11, 2023
Remove the base error message in `OS`, we no longer really error out this
way for not implemented methods. Instead, each platform should override them
to provide the context they want.

Fixes godotengine#82439.
ProbablyWorks pushed a commit to ProbablyWorks/godot that referenced this issue Oct 22, 2023
Remove the base error message in `OS`, we no longer really error out this
way for not implemented methods. Instead, each platform should override them
to provide the context they want.

Fixes godotengine#82439.
akien-mga added a commit to akien-mga/godot that referenced this issue Jan 18, 2024
Remove the base error message in `OS`, we no longer really error out this
way for not implemented methods. Instead, each platform should override them
to provide the context they want.

Fixes godotengine#82439.

(cherry picked from commit 0a10f09)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants