PMKS-Web / PMKSWeb

An educational web-based tool designed for comprehensive 2D linkage analysis developed at WPI (Worcester Polytechnic Institute)
https://pmksplus.com
5 stars 3 forks source link

Link name changes when adding tracer points /changing link shapes #93

Open rkprad opened 1 year ago

rkprad commented 1 year ago

Original Linkage: https://deploy-preview-90--pmks.netlify.app/?0P.iO.K,0.101.AB,Trial1,BC,Trial2,CD,Trial3,A,XYZ1,D,XYZ4,B,XYZ2,C,XYZ3.6a,02N9,011k,0.0b,01Bx,02RN,0.0c,0i-,1wH,0.4d,2aX,0dQ,0.8e,02H7,Yj,0.0f,Bu,0rF,0.0g,0_6,0Rn,0.0h,02QA,3Rf,0.0j,04fB,1JD,0..YRab,Fe,Fe,01nY,01kY,c5cae9,a,b,,.YRcd,Fe,Fe,xn,fS,c5cae9,c,d,,.YRgh,Fe,Fe,01i8,1Vy,c5cae9,g,h,,.YRbcefgj,0,0,01iN,5W,0d125a,b,c,e,f,g,e,j,,bcefg,ej.NRbcefg,Fe,Fe,0-G,0Fu,303e9f,b,c,e,f,g,,.NRej,Fe,Fe,03T9,wz,c5cae9,e,j,,..

There is a link called Trial3.

That changed to CDL when a tracer point was added https://deploy-preview-90--pmks.netlify.app/?00.iO.U,0.111.AB,Trial1,BC,Trial2,CD,Trial3,A,XYZ1,D,XYZ4,B,XYZ2,C,XYZ3.6a,02N9,011k,0.0b,01Bx,02RN,0.0c,0i-,1wH,0.4d,2aX,0dQ,0.8e,02H7,Yj,0.0f,Bu,0rF,0.0g,0_6,0Rn,0.0h,02QA,3Rf,0.0j,04fB,1JD,0.4k,02QA,3Rf,0.0l,30-,4Ki,0..YRab,Fe,Fe,01nY,01kY,c5cae9,a,b,,.YRcdl,Fe,Fe,1eB,1ts,c5cae9,c,d,l,,.YRgh,Fe,Fe,01i8,1Vy,c5cae9,g,h,,.YRbcefgj,0,0,01iN,5W,0d125a,b,c,e,f,g,e,j,,bcefg,ej.YPhk,Fe,0,0,0,,h,k,,.NRbcefg,Fe,Fe,0-G,0Fu,303e9f,b,c,e,f,g,,.NRej,Fe,Fe,03T9,wz,c5cae9,e,j,,..

Perhaps, there could be a flag that keeps track of link name changes. If the user has changed link name, then the new name will remain even if tracer points are added.

AnselChang commented 1 year ago

@Kohmei358 @rkprad this is because currently, custom link ids are stored as a dictionary that maps original link id (which is the concatenation of all the joint ids) to custom id, meaning that when a tracer point is added, the original link id changes, which screws up the dictionary.

There's two possible fixes to this:

  1. Whenever a link is changed so that its id changes, change the corresponding key name in the dictionary as well
  2. Store the custom id as a property of Links themselves, without needing to use a separate service to keep track of names.

The first solution seems like a hacky one, while the second one seems to make more sense unless there's a specific reason names need to be stored as external dictionaries.

KohmeiK commented 1 year ago

@AnselChang Great point! I don't know why I didn't start with Solution 2 to being with. Let's remove the service and migrate over to solution 2.

AnselChang commented 1 year ago

Hmm. Looking at edit-panel.component.html, I can see why the dictionary was used as a quick and dirty solution for this. makes it easy to pass original ID as a string to the component, instead of having to deal with referencing actual typescript joint/link/force objects. Would you still like me to go ahead with option 2 with a bit of refactoring or continue with the dirty fix?

KohmeiK commented 1 year ago

Yeah I thought of accessing it through the edit panel but since you already have access to the active object (the selected object), it wouldn’t be too hard to just say this.activeObjectService.displayname from the html. I’d say go for option 2 unless you think it will take you more than an hour


From: Ansel Chang @.> Sent: Saturday, June 10, 2023 1:51:43 AM To: PMKS-Web/PMKSWeb @.> Cc: Kohmei Kadoya @.>; Mention @.> Subject: Re: [PMKS-Web/PMKSWeb] Link name changes when adding tracer points /changing link shapes (Issue #93)

Hmm. Looking at edit-panel.component.html, I can see why the dictionary was used as a quick and dirty solution for this. makes it easy to pass original ID as a string to the component, instead of having to deal with referencing actual typescript joint/link/force objects. Would you still like me to go ahead with option 2 with a bit of refactoring or continue with the dirty fix?

— Reply to this email directly, view it on GitHubhttps://github.com/PMKS-Web/PMKSWeb/issues/93#issuecomment-1584882063, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEYAKQKTBF722FOJG7DNRIDXKNIB7ANCNFSM6AAAAAAY5N7E7Q. You are receiving this because you were mentioned.Message ID: @.***>

AnselChang commented 1 year ago

Oh right, forgot about active object

AnselChang commented 1 year ago

I think I fixed it. Probably some bugs/something I missed as there were a LOT of things dependent on custom id service. but hopefully small