Modify

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#5561 closed defect (fixed)

wrong node selected after "unglue way"

Reported by: skyper Owned by: cmuelle8
Priority: normal Milestone:
Component: Core Version:
Keywords: unglue node Cc:

Description

Hi

In the last weeks the behaviour of unglue ways did change.
Now the old node gets seleted after unglueing instead of the just created one. This is wrong since I need the new node which is member of the way to be selected.

Thanks for changing back.

skyper

3613

Attachments (1)

fix-unglue-ways-cleanup-code.patch (14.2 KB) - added by cmuelle8 9 years ago.
fix things, but please test thoroughly, before applying - let's hope that there are no regression bugs..

Download all attachments as: .zip

Change History (13)

comment:1 in reply to:  description Changed 9 years ago by bastiK

There are multiple use cases for unglue, with different initial selection. Please describe in more detail what you expect and why.

comment:2 Changed 9 years ago by skyper

If I unglue a way I want to do something with this way. Often I want to move this way or at least the node where they previous joined.
In the past I could do this by unglueing and then moving the new node, but now I unglue and have to select the new node manually, which is not that easy, because after unglueing I have doubled nodes.

I think the node which is part of the previous selected way should be selected and not the old node which is not part anymore.
If I want to unglue a different way(node), I would have selected that other way, but again I can not move it right away, because yet again the wrong node is selected.
If I want to unglue landuse-boundaries for example, I would select the closed way and the nodes I want to move, but after unglueing no node of the way is selected anymore. Therefor I have a lots of doubled nodes, and for moving I have to select them all manually again and be careful about not selecting the wrong ones.

Cheers skyper

comment:3 Changed 9 years ago by bastiK

I think I get it: You have nodes A, B, C and Y, way 1 is [A, B, C] and way 2 is [B, Y]. Now you select way 1 and node B and press G, right?

Indeed, it should always select the node of the previously selected way.

comment:4 Changed 9 years ago by bastiK

Owner: changed from team to cmuelle8

The bug is introduced with r3594.

comment:5 in reply to:  3 Changed 9 years ago by skyper

Replying to bastiK:

I think I get it: You have nodes A, B, C and Y, way 1 is [A, B, C] and way 2 is [B, Y]. Now you select way 1 and node B and press G, right?

That's how unglueing works (simple case)

Indeed, it should always select the node of the previously selected way.

That's what I said. This node is always a new node (id:0).

Thanks for accepting.

comment:6 Changed 9 years ago by bastiK

Here is what's wrong:

Unglue action creates 1 node and puts it at the exact same location as the original one. It still selects the right node (the one from the selected way). However, when you click and drag this node gets deselected, and the other one is dragged instead. This is a side effect of the new selection code by Christian Müller.

Changed 9 years ago by cmuelle8

fix things, but please test thoroughly, before applying - let's hope that there are no regression bugs..

comment:7 Changed 9 years ago by cmuelle8

thx for reporting the bug - it helped cleaning up getNearestNodeOrWay()

debugging the bug showed, that it made more sense to use getNearestNode() and getNearestWaySegment() in both (use_selected) cases of that function and rather defer use_selected handling to those.. once I had this done, I recognized that there was a lot of superfluos code in getNearestNodeOrWay(), since the (use_selected) branches of the two if statements in the function body looked almose identical - i could eliminate the two branches and merged them - getNearestNodeOrWay() is much more readable now, i think..

best regards,
christian

ps: please make sure you use the latest patch (i up'ed it twice within 5 minutes or so, since i forgot to uncomment debug())

comment:8 Changed 9 years ago by bastiK

Resolution: fixed
Status: newclosed

In [3642/josm]:

fixed #5561 (patch by cmuelle8) - wrong node selected after "unglue way"

comment:9 Changed 9 years ago by cmuelle8

Hi Sebastian,

how did you manage to have a smaller set of changes in the diff? the patch I commited was generated by Eclipse, but it consists of many tiny diffs instead of the larger chunks you achieved for the tree commit.

is there some setting in Eclipse I can change to have it generate nicer and more easily understandable diffs?

thx,
christian/cmuelle8

comment:10 Changed 9 years ago by bastiK

You can use any diff software you like:

svn diff --diff-cmd DIFF

where DIFF is the name of the programm. This is probably also configurable in Eclipse, but I don't know, because I don't care. :)

comment:11 Changed 9 years ago by cmuelle8

weird, i thought you used some options like "generate a smaller set of changes" but if that is not the case the "Team" diffing in Eclipse just sucks.. (i don't know if this is due to my version of Subclipse, or if the diffing is done by factory eclipse) but i agree, it's a great waste of time to have a look into this :)

bye

comment:12 Changed 9 years ago by bastiK

see #5609 for a related problem

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain cmuelle8.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.