Skip to content

Allow unicode class names#1574

Merged
dsnopek merged 1 commit into
godotengine:masterfrom
dsnopek:unicode-class-names
Oct 29, 2024
Merged

Allow unicode class names#1574
dsnopek merged 1 commit into
godotengine:masterfrom
dsnopek:unicode-class-names

Conversation

@dsnopek

@dsnopek dsnopek commented Sep 6, 2024

Copy link
Copy Markdown
Collaborator

Depends on PR godotengine/godot#96501

@dsnopek dsnopek added the enhancement This is an enhancement on the current functionality label Sep 6, 2024
@dsnopek dsnopek added this to the 4.x milestone Sep 6, 2024
@dsnopek
dsnopek requested a review from a team as a code owner September 6, 2024 22:25
@dsnopek

dsnopek commented Sep 6, 2024

Copy link
Copy Markdown
Collaborator Author

This will fail tests until the upstream Godot PR is merged

@dsnopek
dsnopek force-pushed the unicode-class-names branch 2 times, most recently from d35fb0e to d8c21f3 Compare September 11, 2024 21:19
@dsnopek
dsnopek requested a review from a team as a code owner September 11, 2024 21:19
@dsnopek

dsnopek commented Sep 11, 2024

Copy link
Copy Markdown
Collaborator Author

Hrm. Compiling locally, the tests work fine. However, on GitHub Actions, both GCC and MSVC don't like the UTC-8 identifiers in the C++ code. I tried setting the LANG and LC_ALL environment variables, hoping that would help with the GCC on Linux builds, but it had no effect.

This will need some more research to figure out!

@Repiteo

Repiteo commented Sep 18, 2024

Copy link
Copy Markdown
Contributor

After some local testing, it appears that the gcc used in CI is the culprit. Changing runs-on to ubuntu-latest eliminates that error entirely. I'm having no luck on identifying where/when this bug was actually fixed, just that the gcc used in ubuntu-20.04 has the bug.

No comment on the CMake equivalents, not familiar enough with that buildsystem.

@dsnopek
dsnopek force-pushed the unicode-class-names branch from d8c21f3 to 3356a35 Compare September 18, 2024 17:48
@dsnopek
dsnopek force-pushed the unicode-class-names branch from 3356a35 to 536ea85 Compare September 18, 2024 18:25
@dsnopek

dsnopek commented Sep 18, 2024

Copy link
Copy Markdown
Collaborator Author

Thanks! I updated to Ubuntu 22.04, which we're using in the static checks already, and that fixes the Linux/GCC builds.

I'm testing out some flags to fix the MSVC build.

@dsnopek

dsnopek commented Sep 18, 2024

Copy link
Copy Markdown
Collaborator Author

Huzzah! The MSVC flag I tried is working, so this is now fully passing tests :-)

@Ivorforce Ivorforce left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything wrong with this, LGTM

@paddy-exe paddy-exe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed at the GDExtension meeting. Changes look good.

@dsnopek
dsnopek merged commit 7fca545 into godotengine:master Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This is an enhancement on the current functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants