Skip to content

Conversation

@bradAnderson58
Copy link

Hello friends,

I happened across a use case similar to this issue here, although emackey speaks of an enum for picking priority, whereas my use case was I suppose more similar to this guys question on the forum who only wanted to be able to toggle on/off selectability of individual Entities.
I created a 'selectable' member for the Entity which will govern whether a user can select an Entity. The selectable defaults to true and can be set in the 'options' object parameter of the Entity constructor, or by setting the 'selectable' value in a loaded CZML object.
TODO:
Write some unit tests. Im kind of a noob so unfamiliar with Jasmine, I will have to read into it this weekend, but I wanted to get feedback if this is functionality is something you want.

Entity now has a 'selectable' member which determines whether the given
Entity is selectable by the user.  'selectable' defaults to true, and can
be set via the 'selectable' field in the options parameter of the Entity
constructor.  Additionally, CzmlDataSource has been updated to handle the
'selectable' field and set an Entities 'selectable' value.
Fixed typos after testing
@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2015

Thanks for this contribution, @bradAnderson58! Can you please send in a Contributor License Agreement (CLA) so we can review this?

jsHint doesn't like my bad coding conventions
@bradAnderson58
Copy link
Author

Sorry I read that on the wiki but then I forgot. Sent.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2015

Thanks @bradAnderson58! We received it.

Added unit tests and also fixed some bad assumptions which were exposed by
existing unit tests
@bradAnderson58
Copy link
Author

Okay sorry I didnt get back to this forever, but there are now unit tests for my changes. Also, I made a small example http://danglingpointers.me/unrelated/brad_cesium/HelloWorld.html
This shows an entity which is selectable by default, an entity which is made explicitly selectable, and an entity which is unselectable. It also loads two entities from CZML, one which is selectable and one which is not

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 4, 2015

Thanks @bradAnderson58. Someone will review this soon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mramato is there a better place to put this higher in the stack?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradAnderson58 the problem here is that id is not always guaranteed to be an Entity instance, so we can't rely on this check here.

We don't have a top-level Entity picking API right now, so this would have to be done in user code. We want to add one though, DataSourceDisplay would probably be a good place for that.

I think the bigger problem is that this can't be implemented efficiently without such functionality at the primitive level. We basically would have to have every primitive have the selectable property, then keep a similar check here. However, I don't think selectable is the right approach for all use cases. I would rather see a pickPriority property as discussed in #1592. It would be a number where <=0 means do not pick and higher numbers means "pick this first". This has drill pick ramifications but shouldn't be too hard optimize for most use cases. If we added this feature at the primitive level, we would then have to thread it through up to the Entity API and visualizers.

If @pjcozzi is on board with changing the Primitive API and you are interested in working on this, I can write up some more details instructions about what I think has to happen.

Also, sorry for taking so long to look at this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am for the pickPriority approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey guys, sorry I took so long to respond to this, the email just got lost in the sauce around the holidays. I would definitely be interested in working on this. I agree that the pickPriority option is preferrable to the selectable. I think I was in a bit of an application-specific mindset there.
I will dig around in the DataSourceDisplay and such to get a better idea of whats going on there while I await instructions.
Again, sorry for taking so long to respond.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bradAnderson58 did you get a chance to look at this?

@hpinkos
Copy link
Contributor

hpinkos commented Dec 7, 2016

This PR has been superseded by #4410, which I think is the approach we want to go with to solve #1592
Thanks again for looking into this @bradAnderson58

@hpinkos hpinkos closed this Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants