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

call_group() does not always call all nodes in the group #44765

Open
db0 opened this issue Dec 28, 2020 · 7 comments · Fixed by #67831 · May be fixed by #69210
Open

call_group() does not always call all nodes in the group #44765

db0 opened this issue Dec 28, 2020 · 7 comments · Fixed by #67831 · May be fixed by #69210

Comments

@db0
Copy link

db0 commented Dec 28, 2020

3.2.3 stable official

OS/device including version:

Windows 10 x64
Issue description:

I have a part in my code, where I want to execute a method on all nodes in a group. The method which includes the call_group() is initiated by a signal from one of the nodes in that group.

In some special circumstance, which I'm not exactly certain about, the call_group() will call the method on all nodes except the node which sent the original signal.

However, I can make the call_group() work with the following two workarounds:

  • Change call_group() to call_group_flags(SceneTree.GROUP_CALL_UNIQUE, ...)
  • Use a for loop like so for node in cfc.get_tree().get_nodes_in_group("group_name"):

Both of the above will correctly execute on all nodes, including the one sending the signal. All other flags for call_group_flags() also fail to execute on the signaling node.

This also works when I trigger the signal in a slightly different set of behaviours. Please see me reproduction info.
Steps to reproduce:

So, this is quite tricky to replicate in a minimal project as it seems to work normally, but I seem to have crafted a situation where I can make it appear only if I trigger the signal with a specific setup.

As I cannot craft a minimal product, I have instead created a branch in my repo which I have prepared so that this bug is immediately reproducible: https://github.com/db0/godot-card-game-framework/tree/GROUP_CALL_UNIQUE-bug

To reproduce:

  1. Clone my git repo and checkout the GROUP_CALL_UNIQUE-bug branch
  2. Run main scene with F5
  3. A hand of 5 blue cards will appear.
  4. Drag a blue card from the hand anywhere on the board. It will be placed in the PlacementGridDemo and 6 "bio" tokens appear correctly.
  5. Drag a blue card from the hand specifically inside the "PlacementGridDemo" area. You will see only 2 "bio" tokens appear. However the code is setup so that 6 should appear. One via each call_group_flags() flag, one via call_group() and one via the for loop.

I have replicated this with simple prints as well, so it's not an issue with the "token adding" code. The execute_scripts() function is really not called at all on the trigger card. This way is just easier to see visually.

The 6 group calls are defined in res://src/core/CFControl.gd Line 234
You can see that they all originate in the same spot and are called in the same manner. But for some reason, 4 out of 6 fail to execute on

Apologies for the complexity of this report, but I couldn't find a better way to report it.

@omicron321
Copy link
Contributor

omicron321 commented Dec 28, 2020

1. Clone and run main scene.

2. A hand of 5 blue cards will appear.

so main scene is... res://src/custom/CGFMain.tscn

  1. there are no such thing as cloning a scene, but many options... duplicate, instance, new inherited scene... etc.
  2. i didn't get that 5 blue cards appearing for options i have tried
  3. n/a

@db0
Copy link
Author

db0 commented Dec 28, 2020

there are no such thing as cloning a scene, but many options... duplicate, instance, new inherited scene... etc.

I meant, git clone the repository, switch to the branch I mentioned and just run the whole project with F5 :) CGFMAin.tcsn should be already set as the main project scene.

i didn't get that 5 blue cards appearing for options i have tried

Did you switch to the GROUP_CALL_UNIQUE-bug branch? This is what I've prepared to replicate the issue

@omicron321
Copy link
Contributor

i could reproduce described issue with your steps
i couldn't build a MWE from scratch from empty scene
so i guess one of step to isolate bug could be to prune everything in that branch that isn't required to reproduce that bug

@db0
Copy link
Author

db0 commented Dec 29, 2020

That's going to be quite tricky as there's a lot of interconnected pieces before one gets to that point. I think this bug appears precisely because of some weird interaction in the whole process.

I do not know how Godot development works, but an alternative would be if one could attach a breakpoint in the C++ code at the point where it's calling the groups,it might point out why the signal card is not being called.

@Calinou
Copy link
Member

Calinou commented Aug 20, 2021

@db0 Can you (or anyone else) still reproduce this bug in Godot 3.3.3 or any later release?

If yes, can you reproduce this after switching to call_group_flags() with the GROUP_CALL_REALTIME flag passed? This disables deferred calling to use immediate calling instead. If this resolves the issue, then #51591 should make things better in this regard 🙂

@db0
Copy link
Author

db0 commented Aug 20, 2021

Tried it just now (same branch as in my report). Issue persists using the GROUP_CALL_REALTIME flag. Only GROUP_CALL_UNIQUE works correctly.

@kleonc
Copy link
Member

kleonc commented Oct 24, 2022

To reproduce:

1. Clone my git repo and checkout the `GROUP_CALL_UNIQUE-bug` branch

2. Run main scene with F5

3. A hand of 5 blue cards will appear.

4. Drag  a blue card from the hand anywhere on the board. It will be placed in the PlacementGridDemo and 6 "bio" tokens appear correctly.

5. Drag a blue card from the hand  specifically inside the "PlacementGridDemo" area. You will see only 2 "bio" tokens appear. However the code is setup so that 6 should appear. One via each `call_group_flags()` flag, one via `call_group()` and one via the for loop.

Can confirm (only 2 "bio" tokens appear). I've opened a PR with the fix, with it applied 6 "bio" tokens appear as expected. Until then a reliable workaround is this same as for #67819.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants