Modify

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#18909 closed enhancement (fixed)

[PATCH] UniqueIdGenerator.generateUniqueId() should be public

Reported by: taylor.smock Owned by: Don-vip
Priority: normal Milestone: 20.03
Component: Core Version:
Keywords: unique id Cc: StephaneP

Description

Currently if I want to decrement and get an arbitrary id, I need to call currentUniqueId and advanceUniqueId sequentially.

Attachments (1)

18909.patch (538 bytes) - added by taylor.smock 3 months ago.
make generateUniqueId public

Download all attachments as: .zip

Change History (9)

Changed 3 months ago by taylor.smock

Attachment: 18909.patch added

make generateUniqueId public

comment:1 Changed 3 months ago by Don-vip

Owner: changed from team to taylor.smock
Status: newneedinfo

I need to ask you why you want to do that.

comment:2 Changed 3 months ago by taylor.smock

I'm making some modifications to the Mapillary plugin right now, and one of the things I am trying to do is reduce the amount of clutter on the screen (specifically, I'm trying to condense Mapillary Images into lines parallel to a way, and then have some kind of popup on click).

In the process of doing this, I decided it would be a good idea to implement INode in MapillaryAbstractImage (I intend to modify the Geometry class to take IPrimitive/INode/IWay/IRelation, if at all possible instead of OsmPrimitive/Node/Way/Relation, but I think that is going to have to wait until after the next stable).

Mapillary ids are mixed letters/numbers, which means I need to do something else to implement getId which is supposed to return a Long.

The easiest method would, technically, be to return 0 for getId, but I'd rather have a unique long for each image. I intended to extend UniqueIdGenerator so I could have a map of Mapillary keys to longs, to ensure that each key mapped to one and only one long. (I haven't done this yet.)

Finally, it kind of struck me as odd that the method for generateUniqueId was protected when the other methods were not, and I could synthesize the effect of generateUniqueId with the two other calls.


Hopefully that answered your question and was relatively clear.

comment:3 Changed 3 months ago by Don-vip

Cc: StephaneP added
Milestone: 20.03
Owner: changed from taylor.smock to Don-vip
Status: needinfoassigned

It's clear enough, thanks. I made initially this method non public as I feared to allow plugin developers to mess with primitive ids while I didn't see any valid use case for it. You gave me one. What you are describing sounds a lot like what is asked in #18434.

comment:4 Changed 3 months ago by Don-vip

Resolution: fixed
Status: assignedclosed

In 16108/josm:

fix #18909 - UniqueIdGenerator.generateUniqueId() should be public (patch by taylor.smock)

comment:5 Changed 3 months ago by taylor.smock

Actually, that is pretty much what I just finished doing in the Mapillary plugin. I haven't made any pull requests yet, and it will definitely need another set of eyes on it (it is a rather large commit).

Now that you've mentioned #18434, I'll see if I can modify the Mapillary plugin to use GeoImage/GpxImageEntry, and make the appropriate modifications for a OsmData class for images.

comment:6 Changed 3 months ago by Don-vip

Taylor, I think if you come to SOTM France this summer, Stéphane will offer you a lot of beers!

comment:7 Changed 3 months ago by taylor.smock

I'm not currently scheduled to go to any conferences. But if I do finish the feature, it is still going to require some modifications in the various tools.

comment:8 Changed 3 months ago by StephaneP

I think I understand what this patch makes possible....Thank you Taylor!!!!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
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.