Skip to content
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

CallbackProperty and positions #8944

Open
mramato opened this issue Jun 10, 2020 · 12 comments
Open

CallbackProperty and positions #8944

mramato opened this issue Jun 10, 2020 · 12 comments

Comments

@mramato
Copy link
Contributor

mramato commented Jun 10, 2020

We've has several users state that they are using CallbackProperty for entity.position. This is not officially support and only works "by accident" if you are always specifying ECF coordinates and not using path visualization.

However, it is not possible to do this in TypeScript because the types do not match up. We should probably just add a CallbackPositionProperty to handle this use case officially and call it a day.

@samherrmann
Copy link

I'm not sure if this should be a separate issue, but based on the sandcastle example, line 43, it should be possible to assign a Cartesian3 position to entity.position. This does not currently appear to be the case. Entity.position is typed as PositionProperty, which in the TypeScript definition is incompatible with Cartesian3.

@mramato
Copy link
Contributor Author

mramato commented Jun 26, 2020

@samherrmann TypeScript does not allow multiple types for properties, so while Cesium via JS allows you to use Cartesian3 and other primitive types, TS does not. You can read through this comment for the details: #8898 (comment)

Instead, you need to use entity.position = new ConstantPositionProperty(cartesian3).

We plan to eventually make TS expose Property types as generics, but haven't had time to work on it yet.

@benwiles1
Copy link

Couldn't you use union types? eg position: Cartesian3 | PositionProperty not sure what that would look like in jsdoc.

@samherrmann
Copy link

samherrmann commented Jul 7, 2020

@benwiles1 I was thinking that too at first, but the problem with that is that when getting a value it would be typed as Cartesian3 | PositionProperty while in reality is always of type PositionProperty. Consequently, to make TypeScript happy, you'd have to do a type check before being able to access the PositionProperty value. So personally I don't think that approach would improve on the current situation where you have to wrap the value in a ConstantPositionProperty when setting it.

Edit

To put my words above into examples...

Type of entity.position is PositionProperty:

// set position:
entity.position = new ConstantPositionProperty(myCartesian3);
// get position:
entity.position.getValue(...);

VS

Type of entity.position is Cartesian3 | PositionProperty:

// set position:
entity.position = myCartesian3;
// get position:
if (entity.position instanceof PositionProperty) {
  entity.position.getValue(...);
}

@thw0rted
Copy link
Contributor

Good news re: the Typescript discussion. It looks like microsoft/TypeScript#42425 is finally on the way, so it'll be possible to type all of these as get: PositionProperty, set: Cartesian3 | PositionProperty.

@maxizrinUX
Copy link

There is a simple workaround for this:

// Create new Entity, with no position.
const entity = new CesiumEntity({billboard: img});
// Add the position callback.
(entity as any)["position"] = new Cesium.CallbackProperty(() => cartesian, false);

@thw0rted
Copy link
Contributor

If you're going to anycast to get around the type restrictions, you might as well just do new Entity({billboard: img, position: new CallbackProperty(() => ...) as any}).

Eventually, I agree with Matt from the OP, there should be a CallbackPositionProperty which just implements both interfaces.

@EjiHuang
Copy link

EjiHuang commented Sep 5, 2022

I used "@ts-ignore" to fix this. It's not a good idea.

//@ts-ignore
pickedEntity.position = new Cesium.CallbackProperty(() => cartesian, false);

@mholthausen
Copy link

Will there be a CallbackPositionProperty in the near future?

@dukeofcool199
Copy link

what is the status of this issue? I am currently running cesium version 1.102.0 and am trying to get my entities to move using a callbackProperty. Even after forcing the type as any the behaviour I get is that cesium will silently crash. Was the accidental nature of the callback changing entity positions patched at some point?

@ggetz
Copy link
Contributor

ggetz commented Nov 6, 2023

We don't have immediate plans to work on this item. But we'd be happy to accept a Pull Request for this feature if you have the time! Thanks for your interest!

@angrycat9000
Copy link
Contributor

I ran into this and have been using @ts-expect-error to work around this.

  const position = new Cartesian()
  // @ts-expect-error https://github.com/CesiumGS/cesium/issues/8944
  entity.position = position;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants