Three.js: Consolidating MapControls, OrbitControls ans TrackballControls

[mrdoob]

Continuing the discussion from #18483.

The problem with that approach is that, if we did that, why not also consolidate all loaders into a single file too?

For usability reasons I think it's better to keep this API:

import { MapControls } from './jsm/controls/MapControls.js';
import { OrbitControls } from './jsm/controls/OrbitControls.js';
import { TrackballControls } from './jsm/controls/TrackballControls.js';

So the proble to solve is how to make these files consolidate code without changing the API.

[sciecode]

There are two differences between doing the proposed changes and consolidating all loaders into a single file.

1. We are not trying to consolidate every control into a single one. Just the ones where the implementation is so similar that it makes sense maintaining a unified and clear code. MapControls is, for the most part, exactly as OrbitControls but with the ROTATE and PAN buttons swapped. TrackballControls only deviates in that we don't keep the camera.up fixed.

2. Loaders have many sub-dependencies and differing workflows. If it makes sense to unify some loaders, I'm all for it. But I don't think many are similar enough where it would be beneficial maintaining and delivering a single file.

We can avoid multiple changes in the API if we serve the base class from /src and serve each the individual extension class from their own file.

If we want to keep serving every class from examples, we need to export them from a single file for the time being, otherwise we create an unwanted dependency on /js extension classes.

---

As a side note, I think Loaders share enough similarity in parsing data that we could provide a DataParser.js utility class for handling this specific task. Some loaders already have something similar implemented. We should discuss this on a different thread.

But the reason why I mention and haven't filed a PR for this inclusion is because it has similar problems to what we are discussing. DataParser is essentially a utility class for example loaders, but we can't serve it on the /examples folder or we'll have problems "importing" on /js/loaders/.

[mrdoob]

One additional note: three.min.js is already too big and I really try to avoid code moving to core when possible.

[Mugen87]

DataParser is essentially a utility class for example loaders, but we can't serve it on the /examples folder or we'll have problems "importing" on /js/loaders/.

That is good point. I think we can easier implement such refactoring as soon as exmaples/js is gone. How about waiting with this issue until we git rid of exmaples/js? In this way, we can head for a cleaner solution by utilizing the ES6 module magic.

One additional note: three.min.js is already too big and I really try to avoid code moving to core when possible.

In this case, it seems not appropriate to move CameraControl into the core. However, we might want to investigate if a generic Controls class could be developed which serves as a base class for all controls. A common interface/behavior for stuff like connect(), disconnect(), dispose() and event handling might be interesting.

[randName]

I think option (A) would work

i.e. have an index.js in the controls folder that just imports each class and re-exports them

import { MapControls } from './jsm/controls/MapControls.js';
import { TrackballControls } from './jsm/controls/TrackballControls.js';
// ... other imports
export { MapControls, TrackballControls, /* ... */ }

user code:

// bundler
import { MapControls, TrackballControls } from 'three/examples/jsm/controls';

// web
import { MapControls, TrackballControls } from 'three/examples/jsm/controls/index.js';

[mrdoob]

The problem is that FirstPersonControls may not reuse the code that MapControls, OrbitControls and TrackballControls aims to reuse.

[randName]

The problem is that FirstPersonControls may not reuse the code that MapControls, OrbitControls and TrackballControls aims to reuse.

I was going to say that the index.js doesn't actually merge anything and if there was some base utilities it can be pulled out if needed...

but i'm assuming that this is a problem with having to download multiple files to get a single type of control? e.g. having to download both CameraControls.js and MapControls.js if MapControls inherits stuff from CameraControls

IIRC for SVGRenderer, it depends on Projector in the same folder, so I had to get both files (didn't clone the entire repo)

[paulmasson]

One additional note: three.min.js is already too big and I really try to avoid code moving to core when possible.

@mrdoob are you opposed in principle to having some generic controls as part of the core code? As long as you provide three.min.js there will be people, including myself, who prefer using it via vanilla JS. I should think controls of some sort would be essential to just about everyone and worthy of inclusion in the core.

[mrdoob]

@mrdoob are you opposed in principle to having some generic controls as part of the core code?

Until we're able to trim core, that'll make core bigger. So yeah, I'm opposed until then...