Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.4-dev6: Signals don't work in exported builds if .godot dir was created by dev6 #100097

Closed
mgiuca opened this issue Dec 6, 2024 · 12 comments · Fixed by #100235 · May be fixed by #100160
Closed

4.4-dev6: Signals don't work in exported builds if .godot dir was created by dev6 #100097

mgiuca opened this issue Dec 6, 2024 · 12 comments · Fixed by #100235 · May be fixed by #100160

Comments

@mgiuca
Copy link

mgiuca commented Dec 6, 2024

Tested versions

  • Reproducible in: v4.4.dev6.official [1f47e4c]
  • Not reproducible in: v4.4.dev5.official [9e60984]

System information

Godot v4.4.dev6 - Ubuntu 24.04.1 LTS 24.04 on X11 - X11 display driver, Multi-window, 2 monitors - OpenGL 3 (Compatibility) - NVIDIA GeForce GTX 1060 6GB (nvidia; 535.183.01) - Intel(R) Core(TM) i5-4570 CPU @ 3.20GHz (4 threads)

Issue description

In 4.4-dev6, signals do not work, at all in exported builds. The signal handlers are never called. I have tried both built-in signals (such as clicking a button) and custom signals (calling .emit()).

However, the circumstances are extremely weird. It is not tied to the Godot version you used to export, but rather, the Godot version that created the .godot directory.

To elaborate:

  • If you create a new project in 4.4-dev6, then export it, signals are broken.
  • If you create a project in 4.4-dev5, then upgrade to 4.4-dev6, then export, signals work.
  • However, if you delete the .godot directory, then re-open with 4.4-dev6, signals are broken.
  • But, if you then re-open the project (with the .godot dir created in dev6) in 4.4-dev5, and export, signals are still broken!

So this bug can manifest in 4.4-dev5 and earlier, but only if the .godot dir was created by dev6.

I'm at a loss to explain this behaviour, but I have tested it extensively (on Linux), both on a "real" project and in a minimal repro. I have compared the "good" and "bad" .godot directories and can't see much that would distinguish them - the binary .scn files are different but you would expect those to regenerate every time you re-save the scene. And shader cache files are different, but you wouldn't expect that to affect something as unrelated as signals.

And to be clear, everything works fine in the editor. It's only broken for exported builds.

I have tested both the Linux native and Web exports, and the issue affects both.

Steps to reproduce

  1. Create a new project using 4.4-dev6.
  2. Create a new "User Interface" scene.
  3. Attach a script to the scene.
  4. Add a Button to the scene and give it some text.
  5. In the editor, connect the Button's pressed() signal up to a new method.
  6. Add the code print('Button pressed') to the signal handler. For good measure, also $Button.text = 'Pressed'.
  7. Run the project (setting the scene as the main scene) and verify that clicking the button prints "Button pressed" to the console and changes the button text.
  8. Project > Export and add a Linux native export, without changing any settings.
  9. Export Project. Ensure that "Export With Debug" is ticked.
  10. From the command line, run New Game Project.x86_64.
  11. Observe that clicking the button has no effect, neither printing to the console, nor changing the text.

As mentioned above, there are some very weird interactions if we change Godot versions:

  • Open the project in dev5 - it is still broken.
  • Delete the .godot dir and re-open in dev5 - it now works.
  • Open the project in dev6 - it still works.
  • Delete the .godot dir and re-open in dev6 - it is now broken again.

Minimal reproduction project (MRP)

signal-repro.zip

@mgiuca
Copy link
Author

mgiuca commented Dec 6, 2024

Just looking at the dev6 changelog, a possible commit that might have caused this is #97303. (Because perhaps the "avoid duplicating signals" avoided something necessary to do with signals, and wrote that into the PackedScene inside the .godot directory, which means signals don't fire even if it's loaded in old versions.)

@adamscott adamscott added this to the 4.4 milestone Dec 6, 2024
@adamscott adamscott moved this from Unassessed to Immediate Blocker in 4.x Release Blockers Dec 6, 2024
@adamscott
Copy link
Member

I tested the issue (on the Web platform, running dev5 and dev6 as explained) and I can confirm it.

@adamscott
Copy link
Member

I can confirm too that the regression is caused by #97303. I reverted the PR from master, then created a project with it. And it works when exported.

@akien-mga
Copy link
Member

CC @cixil

@KoBeWi
Copy link
Member

KoBeWi commented Dec 6, 2024

The problem seems to be how the CONNECT_INHERITED is applied.

cfrom->connect(snames[c.signal], callable, CONNECT_PERSIST | c.flags | (p_edit_state == GEN_EDIT_STATE_MAIN ? 0 : CONNECT_INHERITED));

Changing this to use GEN_EDIT_STATE_MAIN fixes the bug, but it's probably not the right fix.
Node *node = ps->instantiate(PackedScene::GEN_EDIT_STATE_INSTANCE); // Make sure the child scene root gets the correct inheritance chain.

The bug happens, because scenes are processed during export if you have editor/export/convert_text_resources_to_binary enabled, and due to wrong inheritance state, the connections are lost.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 7, 2024

Well there is another case of this bug (or at least I think it is related). Signal connections don't work in scenes instantiated by plugins, like custom main screens.

EDIT:
Ok confirmed it to be the same problem.

EDIT2:
It's actually caused by my specific way of creating some nodes. I pack a scene and then instantiate it multiple times.

EDIT3:
These might be related: #48064 #85372 #85606

@mr-dreich
Copy link

Don't know if it matters, but you can get around this bug by connecting signals via code instead of the UI.

@Kimau
Copy link
Contributor

Kimau commented Dec 9, 2024

Confirmed on android XR builds. Good to know ^_^

@cixil
Copy link
Contributor

cixil commented Dec 9, 2024

Can anyone explain exactly what the role is for this flag:

CONNECT_INHERITED = 16, // Whether or not the connection is in an instance of a scene.

and these enums?:

enum GenEditState {
GEN_EDIT_STATE_DISABLED,
GEN_EDIT_STATE_INSTANCE,
GEN_EDIT_STATE_MAIN,
GEN_EDIT_STATE_MAIN_INHERITED,
};

re @KoBeWi's comment
I don't quite understand why the inherited flag would be added during instantiation, isn't this flag just supposed to be a way for the editor to determine which connections are a part of the scene?

cfrom->connect(snames[c.signal], callable, CONNECT_PERSIST | c.flags | (p_edit_state == GEN_EDIT_STATE_MAIN ? 0 : CONNECT_INHERITED));

EDIT: Ah I see this is added when instantiated scenes are added as children in the editor? I forget the same code is used in the game runtime and in the editor

@KoBeWi
Copy link
Member

KoBeWi commented Dec 9, 2024

The flag is temporary. All connections that belong to a different scene than the currently opened one will get the flag once the scene is loaded; this applies to instances and inherited scenes.

@cixil
Copy link
Contributor

cixil commented Dec 9, 2024

I guess a simple fix to #97303 could be to add #ifdef TOOLS_ENABLED around the check for the inherited flag (haven't tested). But I'm not sure if this issue points to an incorrect use of CONNECT_INHERITED that should be fixed instead.

@KoBeWi
Copy link
Member

KoBeWi commented Dec 9, 2024

The problem exists in the editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment