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

Signals are not duplicated with their parent node when created from editor UI #1888

Closed
kubecz3k opened this issue May 12, 2015 · 9 comments
Closed

Comments

@kubecz3k
Copy link
Contributor

Bugsquad edit: Initially seen in 1.1 rc2 and still exists in 2.0 alpha b2f9acb commit

Update2 (20.01.2016)
One more time, a better test project to test the issue: DuplicateTests.zip

Update:
New sample project that replicate this issue in more straightforward way:
https://dl.dropboxusercontent.com/u/168251/godot/bugsquad/NotDuplicatedSignalIssue.7z
Also small video that shows how issue has been replicated in this sample (for quick judge):
https://youtu.be/_fYBUKBWzrA

Old (not so relevant currently) content:
It seems that VisibilityEnabler/Notifer is not working if it's on a scene that has been duplicated with .duplicate() method. Below is a link to sample project that illustrates the problem.
https://dl.dropboxusercontent.com/u/168251/godot/some/issues/DuplicateIssue.7z

@kubecz3k kubecz3k changed the title (Bug) VisibilityEnabler don't work when duplicated [1.1 rc2] [1.1 rc2] [Bug] VisibilityEnabler don't work when duplicated May 12, 2015
@kubecz3k
Copy link
Contributor Author

kubecz3k commented Aug 8, 2015

It looks like this bug is caused by the fact that signals are not duplicated (if they were set from gui).

@akien-mga akien-mga changed the title [1.1 rc2] [Bug] VisibilityEnabler don't work when duplicated VisibilityEnabler don't work when duplicated Nov 16, 2015
@kubecz3k kubecz3k changed the title VisibilityEnabler don't work when duplicated Signals are not duplicated with their parent node. Nov 17, 2015
@kubecz3k
Copy link
Contributor Author

Changed title to better reflect actual issue.
I can confirm it still happens in Godot 2.0 alpha b2f9acb
Here is new test case that test the issue in more straightforward way:
https://dl.dropboxusercontent.com/u/168251/godot/bugsquad/NotDuplicatedSignalIssue.7z

@kubecz3k kubecz3k changed the title Signals are not duplicated with their parent node. Signals are not duplicated with their parent node when created from editor UI Nov 17, 2015
@neikeq
Copy link
Contributor

neikeq commented Jan 20, 2016

This was fixed by the above PR, but was not closed automatically.

@kubecz3k
Copy link
Contributor Author

Unfortunately the fix was only partial, signals that are emitted by internal (not scene root) nodes are still lost during duplication.
I have prepared small project that tests various things related to duplication method and report results in the following form:

----Testing signal duplication----
Root signal duplicate status: SUCCESS
Inner signal duplicate status: FAIL

----Testing group duplication----
Root node group duplicate status: SUCCESS
Inner node group duplicate status: SUCCESS

----Testing dynamic structure duplication----
Translation test: SUCCESS
Root export variable test: SUCCESS
Inner export variable test: SUCCESS
Root variable value test: FAIL
Inner variable val test: FAIL
Inner node structure test: SUCCESS

------- Final result------- 
Summary result: FAIL

Here is projects for testing these things:
DuplicateTests.zip

@kubecz3k
Copy link
Contributor Author

Related issue but with variable values: #3393

@vnen
Copy link
Member

vnen commented Jan 20, 2016

There might be a problem if someone actually wants a shallow copy. Recursively copying all variables, signals, groups, and stuff will create an overhead if the programmer does not want to duplicate the state, but only the structure. IMO this should be optional and default to a shallow copy.

@kubecz3k
Copy link
Contributor Author

@vnen I assume signals can be safety duplicated under the condition that both emitter and receiver are inside duplicated node. Currently it don't work even if emitter and receiver is the same node.
When it comes to copying internal variables there is another issue about this but generally I think it should be on by default (exported variables are copied currently after all), with eventual possibility to create your copy manually via duplicate() method overriding (instead of using build one mechanism).

@akien-mga
Copy link
Member

I think this was fixed recently, can you confirm?

@akien-mga
Copy link
Member

Tested with the above-linked project and signal duplication seems to work fine now:

----Testing signal duplication----
Root signal duplicate status: SUCCESS
Inner signal duplicate status: SUCCESS

----Testing group duplication----
Root node group duplicate status: SUCCESS
Inner node group duplicate status: SUCCESS

----Testing dynamic structure duplication----
Translation test: SUCCESS
Root export variable test: SUCCESS
Inner export variable test: SUCCESS
Root variable value test: FAIL
Inner variable val test: FAIL
Inner node structure test: SUCCESS

------- Final result------- 
Summary result: FAIL

The issue about the failed test for root and inner variable values is likely #3393, so closing this one as fixed.

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

No branches or pull requests

4 participants