-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Missing call to replaceParent in group.js? #640
Comments
I'll take a look, but this bug could be related to how you mix and matched imports in this bug |
I was able to workaround that issue by creating a "detached" group (i.e. not attached to the scene) by using the following code (more or less): const twoInstance = new Two({/* options go here */});
const dummy_group = new Group(); // detached group to ignore duplicate two-0
for (let k = 0; k < N; k++) {
const g = new Group();
g.addTo(twoInstance.scene);
} |
This simple demo seems to work fine: https://codepen.io/jonobr1/pen/VwQdNjx?editors=0010 |
Thank you for confirming that it works with your example. To understand how TwoJS Group works on my actual project, I added Also, while debugging the issues in my project, I also noticed that adding To compare my biggish project with your example, I also wrote an example similar to yours using Parcel + TypeScript and I confirmed that on this simple example, So, if you could briefly explain how these events are used by TwoJS, it would help me a lot in debugging the real bugs that are hidden in (other parts of) my project. |
Makes sense. The Perhaps if you're calling a lot of operations at once, this event emission gets misinterpreted and the order gets screwed up. For example, I'm thinking some commands like this: var group = new Two.Group();
group.push(a, b);
group.splice(1, 1);
group.shift();
group.push(b); Also, The successive calls could screw with what should be operated on because Hope this helps explain things. It certainly highlights an inconsistency I've been blind to. |
Ah, thank you so much for explaining the inner working of the TwoJS events. After reading your explanation, I start to have a guess why those TwoJS events are not triggered in our project. Earlier in this conversation thread, I mentioned that our project uses VueJS. By the way this project is a collaboration between @dickinson0718 and me. I included his name because he recently submitted the following issues/enhancement requests: We are grateful that you have been very responsive in our communications. Since VueJS also overrides the behavior of the array functions: I'd like to hear your idea for making this work under our VueJS project (assuming my assumption about the events not being triggered). Also, I'd like hear your input on my temporary solution below:
whether these modifications may have negative impact on the other TwoJS functionalities. |
Hmm, interesting. I need to look into how Vue.js alters an Array, but if you look at Here's a question for you, are you setting and/or modifying the children property? |
No, my code does not directly modify the |
I wish I could easily scale down our app to reproduce the issue and post the scaled down version online. However, it has too many moving parts (VueJS 2.x, Vuetify 2.x, Pinia, and TwoJS itself). We did not see this issue when using TwoJS 0.7, but only after upgrading to 0.8. The main reason upgrading to 0.8 is the new |
Thanks again for your commitment. I went through and totally rewrote the Replace your npm install with |
Thank you for your extra effort of rewriting |
🤔 any chance you could give me access to the project then? Where I could reproduce it locally on my end? At this point, I'm just really curious. |
Yes. of course. Our source code is on GitHub: https://github.com/dulimarta/spherical-easel. In particular there is branch In commit 251931 we are using TwoJS 0.8.10. When I use my own TwoJS Fork, the app works as expected. |
Thanks for sharing. I'm using |
Never saw that error before. I use NodeJS 16.13.2 and NPM 8.1.2. Also instead of |
Small update Some things I noticed while trying to debug:
I think I was able to find the problem. For some reason the way you're declaring and setting I'm going to try to keep poking around to understand why it is like that. |
Thank you for spending time debugging our code and looking into the source code and giving us feedback. After following the link and reading your response above where you referred to specific statements, I assume that you are reading/debugging our code from the |
I was using the |
Describe the bug
Two.shape
objects are not inserted correctly to the desired group when done from a mouse handling function. Instead, they are inserted as an immediate child of the<svg>
node.To Reproduce
I'm not sure how to describe the simplified steps to reproduce the bug by trimming down our VueJS project which currently has 350+ files. But I believe the bug shows up whenever a new
Two.Shape
is added to a group from a mouse event handling.My attempt to fix
So, instead of describing how to reproduce the bug, I will describe the changes I made to
group.js
that seems to fix the issue for me. I found this fix after debugging theadd()
function ingroup.js
and noticed that thereplaceParent()
is not always called.The following two functions
add()
andremove()
are ingroup.js
(version 0.8.9)Expected behavior
Objects are inserted into the desired group. Adding the call to
replaceParent()
fixes the issue for me.Screenshots
Environment (please select one):
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: