Do not replace starting digit with underscore when making identifier

[theraot]

This pull request modifies validate_identifier to add an underscore when the String stars with a digit (instead of replacing the digit with an underscore, which was the old behavior). The validate_identifier test cases has been updated accordingly.

Since validate_identifier is modified, the helper method _is_valid_identifier_bit is not necessary, to remove it is_valid_identifier was also modified to not use it.

Fixes #82773

[Calinou]

Tested locally, it works as expected. Results from drag-and-dropping from the FileSystem dock while holding Ctrl:

const _12345 = preload("res://12345.tscn")
const _0 = preload("res://0.gd")
const __ = preload("res://__.tscn")
const _ = preload("res://_.tscn")

validate_identifier() is only used in the script editor and for script template creation. The method isn't exposed to scripting, so it should be fine to cherry-pick for 4.1 (given the small behavior change).

[Calinou]

I'll miss that comment. 👀

[akien-mga]

I'm a bit worried about the impact this may have on compatibility, notably for the import pipeline. We already broke compat in 4.1 by changing the logic that would remove . from node names, and replaced them instead with _. If this method is used similarly, we could again run into a situation where Godot 4.1 imported scenes will break.

CC @godotengine/import to help assess this.

[theraot]

@akien-mga

If it helps, searching on my end, it seems that validate_identifier (aside from the tests) is only used in:

• script_text_editor.cpp on _get_dropped_resource_line (link) and ScriptTextEditor::drop_data_fw (link), i.e. as result of dropping something in the text editor, and generating source code.
• gdscript_editor.cpp on GDScriptLanguage::make_template (link), i.e. also generating source code.

Thus, it does not provide names for nodes. Modifying it should not affect existing scripts. And as far as I know it is not involved in importing anything.

I also modified is_valid_identifier, however, I believe I didn't change its behavior (its test cases are untouched).

At least, I don't expect issues, but of course I might be mistaken.

Addendum: By searching on github (to find the links) I also found this: #67701

[akien-mga]

Sounds good, thanks for researching it. I expected it to be used way more, but I guess we have other similar methods in Node that partially duplicate this functionality.

[akien-mga]

Thanks!

[YuriSizov]

Cherry-picked for 4.1.4.