Skip to content

Conversation

ColinWttt
Copy link
Contributor

Private GdSelfObj cause signal test failed

@Yarwin
Copy link
Contributor

Yarwin commented Mar 1, 2025

https://github.com/Yarwin/gdext/actions/runs/13603957943

Which test? In which CI job? How does changing visibility helps with this issue?

I am unable to replicate this neither in the CI nor locally.

@Bromeon
Copy link
Member

Bromeon commented Mar 1, 2025

Background: moving GdSelfObj into its own module causes problems because the generated signal methods signals().xy() are no longer accessible.

I created the WIP branch bugfix/signal-visibility trying to address this, but there are some hard problems:

  1. The signal generating proc macro #[godot_api] doesn't know about the visibility of the class struct, since that one is handled by a different proc macro #[derive(GodotClass)].
  2. The xy() methods cannot be "more public" than the surrounding class, because:
    • Each such method returns a distinct type __godot_Signal_C_Xy
    • Each such type has a Deref to TypedSignal<'c, C, Ps>
    • The C in TypedSignal is the surrounding class
    • If xy() is public, then __godot_Signal_C_Xy must be public, and all generic args of TypedSignal<'c, C, Ps> must be public
  3. If C (surrounding class) is private, the code won't compile due to "cannot leak private type in public function" style errors.

Adding a pub in the test just works around the problem, it still won't work for private types. As such, this PR unfortunately doesn't solve anything.

We need to address this differently -- maybe there's a neat trick with modules or so, but it might also be that users would have to explicitly specify the visibility in #[godot_api].

@Bromeon Bromeon closed this Mar 1, 2025
@ColinWttt ColinWttt deleted the fix-singal-itest branch March 18, 2025 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants