Circular dependency / global variable causing bug ?

[jailln]

Your Environment

• Version used: a branch of my fork based on the current master
• Browser Name and version: Firefox 75.0 and Chromium 81.0.4044.122
• Operating System and version (desktop or mobile): Ubunut 18.04
• Link to your project: the branch in question

Context

While rebasing one of my branch I came across a bug that does not seem directly related to the code I introduced/modified.

On this branch, all the examples of iTowns produce the following error (while I have not modified Renderer/OBB.js nor Core/Prefab/Globe/BuilderEllipsoidTile.js):

Steps to Reproduce (for bugs)

1. git clone https://github.com/jailln/itowns.git
2. cd itowns
3. git checkout 3dtiles_refactoring-rebase
4. npm install
5. npm start
6. open any example in your browser

Expected Behavior

Examples not related to 3DTiles should work since I only modified 3DTiles related code.

Actual Behavior

All examples are broken.

Possible Cause

By looking into the code I think this error is caused by a combination of:

• the circular dependency between Renderer/OBB.js and Core/Prefab/Globe/BuilderEllipsoidTile.js
• the initialization of the global variable builder which uses BuilderEllipsoidTile's constructor.

However, I don't know why it works on the master branch and why it does not on my branch...

Possible Fix/Solution

Initializing builder to undefined or moving its declaration into the setFromExtent() method (where it is used) solves this issue. However, since I am not familiar with this part of the code, I will let you decide what is the best solution to handle this issue. In addition, is there any reason for this circular dependency and for using global variables in Renderer/OBB.js ?

[zarov]

A better solution, that seems to work on at least a few example: removing the circular dependency. I removed the OBB import in BuilderEllipsoidTile, as the only method on it serves no purpose imho.

I submitted the fix in #1382

[jailln]

Yep that's better! Thanks for your quick response and fix :+1:

[zarov]

The fix is now in master.