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

Deprecations #182

Merged
merged 11 commits into from
Oct 25, 2019
Merged

Deprecations #182

merged 11 commits into from
Oct 25, 2019

Conversation

markpet49
Copy link
Member

Description of the Change

Code cleanup, fixes numerous lint issues and deprecations.

Why Should This Be In Core?

Improves code readability and removes reliance on outdated code.

@markpet49 markpet49 added bug enhancement nasa NASA submitted change labels Oct 16, 2019
@markpet49 markpet49 requested a review from Beak-man October 16, 2019 19:18
@@ -721,9 +721,8 @@ protected Object createLineRenderable(Iterable<? extends Position> positions, St
{
Path path = new Path(positions);
path.setPathType(pathType);
path.setFollowTerrain(true);
path.setSurfacePath(true);
Copy link
Member

Choose a reason for hiding this comment

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

What's the reasoning behind this change? Isn't the setAltitudeMode usage more illustrative of the different modes that a Path may use?

Copy link
Member

@Beak-man Beak-man Oct 23, 2019

Choose a reason for hiding this comment

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

I see that this pattern is changed in other classes as well, for instance the classes in the milstd2525 folder.

Copy link
Member Author

Choose a reason for hiding this comment

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

Polyline (deprecated) and Path (which replaces it) have different treatments of followTerrain. The Path class was designed several years ago and I didn't want to modify its behavior.

I initially replaced polyline.setFollowTerrain with path.setFollowTerrain, but noticed that the behavior was different. Polyline implicitly also applies clamp_to_ground behaviour with followTerrain while path does not.

So rather than replace setFollowTerrain with two calls to achieve the same behavior, I expanded on the isSurfacePath pattern in the Path class to set followTerrain and clamp_to_ground in one call.

path.setPathType(AVKey.GREAT_CIRCLE);
path.setAltitudeMode(WorldWind.CLAMP_TO_GROUND);
Copy link
Member

Choose a reason for hiding this comment

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

the setAltitudeMode function is also used in our other platforms (for instance, in Web WorldWind). Are we sure we want to get rid of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

The call to setSurfacePath on line 499 turns on clamp to ground. setAltitudeMode is still available on the Path class. That being said, please create an issue in WWW to make sure the designs of WWW and WWJ are aligned with respect to Paths and Symbology.

Copy link
Member

Choose a reason for hiding this comment

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

Will do

Copy link
Member

Choose a reason for hiding this comment

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

@Beak-man
Copy link
Member

Beak-man commented Oct 23, 2019

The following examples and apps have been reviewed, covering a little more than two thirds of the files changed in this PR.

Examples:

  • Shapes
  • SurfaceImages
  • FlatWorld
  • Airspaces
  • AirspaceBuilder
  • MGRSGraticule
  • TerrainProfiler
  • PlaceNames
  • Balloons
  • MeasureToolUsage
  • ViewLookAround

Apps:

  • GliderTestApp
  • SARApp (note: This one wasn't thoroughly tested, since it's a complex application on its own, but I ran it successfully and played around with it).

No issues found in these.

Couldn't find an example or app to test the following class, maybe we should consider making one:

  • HTTPFileUpload

@markpet49
Copy link
Member Author

@Beak-man With respect to the HTTPFileUpload class comment, it has been deprecated as of the 2.2 release and I intend to delete it in the release following 2.2. It's not something WWJ needs to provide.

Copy link
Member

@Beak-man Beak-man left a comment

Choose a reason for hiding this comment

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

Every related example was tested, as well as the apps (if superficially so, as is the case with the Search And Rescue app as explained above).

The issues encountered and documented in #183, #184, and #185 are not specific to this PR (they also happen in the develop branch) and appear to be related to their particular examples instead of being issues with the library itself.

@Beak-man Beak-man merged commit ebdc81b into develop Oct 25, 2019
@Beak-man Beak-man deleted the deprecations branch October 25, 2019 17:08
quonn77 pushed a commit to quonn77/WorldWindJava that referenced this pull request May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement nasa NASA submitted change
Development

Successfully merging this pull request may close these issues.

2 participants