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

Rename ModelExperimental files that don't conflict with Model #10494

Merged
merged 7 commits into from
Jul 26, 2022

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented Jun 29, 2022

This PR renames several ModelExperimental files that don't have Model counterparts:

  • ModelExperimental3DTileContent
  • ModelExperimentalAnimationChannel
  • ModelExperimentalArticulation
  • ModelExperimentalArticulationStage
  • ModelExperimentalDrawCommand
  • ModelExperimentalRuntimeNode
  • ModelExperimentalRuntimePrimitive
  • ModelExperimentalSceneGraph
  • ModelExperimentalSkin
  • ModelExperimentalStatistics
  • ModelExperimentalType

For all of these files, the "Experimental" part of the file / class name has been removed.

@cesium-concierge
Copy link

Thanks for the pull request @j9liu!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

@j9liu j9liu requested a review from ptrgags June 29, 2022 20:06
@j9liu j9liu changed the base branch from replace-model-old to replace-model July 1, 2022 20:36
@j9liu j9liu changed the base branch from replace-model-old to replace-model July 21, 2022 14:53
@j9liu j9liu force-pushed the rename-model-experimental-files branch from 269dfd2 to d9bc66e Compare July 21, 2022 15:36
@j9liu
Copy link
Contributor Author

j9liu commented Jul 21, 2022

@ptrgags could you review when you get the chance?

@j9liu
Copy link
Contributor Author

j9liu commented Jul 21, 2022

Found a bug while I was testing (though it's happening on main too):

In the 3D Tiles Next CDB Yemen Sandcastle:

  • Select style buildings based on height
  • Switch to the Airport view
  • Move the camera around the area. Eventually this error will pop up:

image

This is the stack trace:

"Original stack:
Original stack:
Original stack:
Error
    at new DeveloperError (http://localhost:8080/Build/CesiumUnminified/index.js:11257:11)
    at Check.typeOf.number.greaterThan (http://localhost:8080/Build/CesiumUnminified/index.js:11324:11)
    at new MetadataTable (http://localhost:8080/Build/CesiumUnminified/index.js:87532:31)
    at parseFeatureMetadataLegacy (http://localhost:8080/Build/CesiumUnminified/index.js:105204:29)
    at http://localhost:8080/Build/CesiumUnminified/index.js:105350:34
    at async Promise.all (index 0)
Handler stack:
Error
    at new RuntimeError (http://localhost:8080/Build/CesiumUnminified/index.js:13876:11)
    at ResourceLoader.getError (http://localhost:8080/Build/CesiumUnminified/index.js:87690:24)
    at http://localhost:8080/Build/CesiumUnminified/index.js:105366:32
    at async Promise.all (index 0)
Handler stack:
Error
    at new RuntimeError (http://localhost:8080/Build/CesiumUnminified/index.js:13876:11)
    at ResourceLoader.getError (http://localhost:8080/Build/CesiumUnminified/index.js:87690:24)
    at handleError6 (http://localhost:8080/Build/CesiumUnminified/index.js:106215:22)
    at http://localhost:8080/Build/CesiumUnminified/index.js:106201:12
Handler stack:
Error
    at new RuntimeError (http://localhost:8080/Build/CesiumUnminified/index.js:13876:11)
    at http://localhost:8080/Build/CesiumUnminified/index.js:108229:26"

@lilleyse
Copy link
Contributor

lilleyse commented Jul 21, 2022

Possibly related to CesiumGS/cdb-to-3dtiles#69?

N12E045_D300_S001_T001_L6_U52_R1.glb has no meshes

@ptrgags
Copy link
Contributor

ptrgags commented Jul 26, 2022

The renaming looks good, and I searched through the code to make sure nothing was missing.

Thanks @j9liu!

@ptrgags ptrgags merged commit 9e84228 into replace-model Jul 26, 2022
@ptrgags ptrgags deleted the rename-model-experimental-files branch July 26, 2022 16:57
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