Skip to content

Conversation

@Bromeon
Copy link
Member

@Bromeon Bromeon commented May 3, 2025

Following upstream change in godotengine/godot#76560.
Fixes CI which assumed that Node::set_name() would always take a GString argument.

This now needs case differentiation when passing Godot strings to Node::set_name():

  • API level 4.4 can pass set_name(&gstring), set_name(string_name.arg())
  • API level 4.5 can pass set_name(gstring.arg()), set_name(&string_name)

Some thoughts:

  • We could of course implement AsArg for both String/StringName interchangeably, but I didn't do this so far as it would be easy to miss perf-relevant conversions. An API change like this could introduce quite a bit different runtime behavior, which would be very hard to find.
  • Another option would be to provide .arg() even for the same type, to handle explicit "I take potential conversion into account" scenarios.
  • To avoid case differentiations, &str and &String can still be provided as arguments.

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: tooling CI, automation, tools labels May 3, 2025
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1153

@Bromeon Bromeon force-pushed the qol/follow-api-change branch from 0b839b6 to 0d5b577 Compare May 3, 2025 19:06
@Bromeon Bromeon added this pull request to the merge queue May 3, 2025
@Bromeon Bromeon added this to the 0.3 milestone May 3, 2025
Merged via the queue into master with commit a76cda6 May 3, 2025
17 checks passed
@Bromeon Bromeon deleted the qol/follow-api-change branch May 3, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: tooling CI, automation, tools quality-of-life No new functionality, but improves ergonomics/internals

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants