Repository: godotengine/godot
File: core/io/resource.cpp (C++)

Code before:
	// If it's not unique it does not matter because the saver will try again.
	OS::DateTime dt = OS::get_singleton()->get_datetime();
	uint32_t hash = hash_murmur3_one_32(OS::get_singleton()->get_ticks_usec());
	hash = hash_murmur3_one_32(dt.year, hash);
	hash = hash_murmur3_one_32(dt.month, hash);
	hash = hash_murmur3_one_32(dt.day, hash);
	hash = hash_murmur3_one_32(dt.hour, hash);
	hash = hash_murmur3_one_32(dt.minute, hash);
	hash = hash_murmur3_one_32(dt.second, hash);
	hash = hash_murmur3_one_32(Math::rand(), hash);

	static constexpr uint32_t characters = 5;
	static constexpr uint32_t char_count = ('z' - 'a');
	static constexpr uint32_t base = char_count + ('9' - '0');
	String id;
	for (uint32_t i = 0; i < characters; i++) {
		uint32_t c = hash % base;
		if (c < char_count) {
			id += String::chr('a' + c);
		} else {
			id += String::chr('0' + (c - char_count));
		}
		hash /= base;
	}

	return id;
}

void Resource::set_scene_unique_id(const String &p_id) {
	scene_unique_id = p_id;
}

String Resource::get_scene_unique_id() const {
	return scene_unique_id;
}

void Resource::set_name(const String &p_name) {
	name = p_name;
	emit_changed();
}

String Resource::get_name() const {
	return name;
}

void Resource::update_configuration_warning() {
	if (_update_configuration_warning) {
		_update_configuration_warning();
	}
}


Review comment (bug):
I must mention first that all new comments should follow the [comment style guide](https://docs.godotengine.org/en/stable/contributing/development/code_style_guidelines.html#comment-style-guide), ideally. So it should look more like this.
```suggestion
// Warning: there is no validation here.
// Use is_ascii_identifier_char() to validate before passing it in.
// Example: if you pass in a quote, it will corrupt the scene when saving.
```

But more importantly to me, the comment does summarise another reason as to why I was skeptical on exposing this. The outside developer should be warned about this:
- It should be documented in the class reference.
- It should not even be possible to do this. In this situation, a warning should appear, the invalid characters should be replaced or the setter should fail outright. Whatever is the most desired.

This would mean making a distinction between the `set_scene_unique_id` used internally (since the additional checks would be a waste), and the exposed `set_scene_unique_id`.

I believe that in this situation a lot of classes prefix the internal method with an underscore.

Code after:
	// If it's not unique it does not matter because the saver will try again.
	OS::DateTime dt = OS::get_singleton()->get_datetime();
	uint32_t hash = hash_murmur3_one_32(OS::get_singleton()->get_ticks_usec());
	hash = hash_murmur3_one_32(dt.year, hash);
	hash = hash_murmur3_one_32(dt.month, hash);
	hash = hash_murmur3_one_32(dt.day, hash);
	hash = hash_murmur3_one_32(dt.hour, hash);
	hash = hash_murmur3_one_32(dt.minute, hash);
	hash = hash_murmur3_one_32(dt.second, hash);
	hash = hash_murmur3_one_32(Math::rand(), hash);

	static constexpr uint32_t characters = 5;
	static constexpr uint32_t char_count = ('z' - 'a');
	static constexpr uint32_t base = char_count + ('9' - '0');
	String id;
	for (uint32_t i = 0; i < characters; i++) {
		uint32_t c = hash % base;
		if (c < char_count) {
			id += String::chr('a' + c);
		} else {
			id += String::chr('0' + (c - char_count));
		}
		hash /= base;
	}

	return id;
}

void Resource::set_scene_unique_id(const String &p_id) {
	bool is_valid = true;
	for (int i = 0; i < p_id.length(); i++) {
		if (!is_ascii_identifier_char(p_id[i])) {
			is_valid = false;
			scene_unique_id = Resource::generate_scene_unique_id();
			break;
		}
	}

	ERR_FAIL_COND_MSG(!is_valid, "The scene unique ID must contain only letters, numbers, and underscores.");
	scene_unique_id = p_id;
}

String Resource::get_scene_unique_id() const {
	return scene_unique_id;
}

void Resource::set_name(const String &p_name) {
	name = p_name;
	emit_changed();
}