-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
Consolidating MapControls, OrbitControls ans TrackballControls #18496
Comments
There are two differences between doing the proposed changes and consolidating all loaders into a single file.
We can avoid multiple changes in the API if we serve the base class from If we want to keep serving every class from 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. |
One additional note: |
That is good point. I think we can easier implement such refactoring as soon as
In this case, it seems not appropriate to move |
I think option (A) would work i.e. have an 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'; |
The problem is that |
I was going to say that the 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 IIRC for |
@mrdoob are you opposed in principle to having some generic controls as part of the core code? As long as you provide |
Until we're able to trim core, that'll make core bigger. So yeah, I'm opposed until then... |
I just spent several hours having problems with a broken TrackballControls (#21548) and I am a bit amazed that there already was a "solution" available but it have not been resolved yet due to bad teamwork? Since I read through some of the discussion I will add my opinion on the topic. My belief is that it is better to get things rolling than procrastinate on something. It is better to have a working implementation that can be used. It does not need to be a perfect solution on first try as long as the code gets tested. The perfect solution usually comes after a few iterations when the developers starts to understand the problems and comes up with good solutions. Without a working solution no one will be able to understand to full complexity of a given problem. There are two ways to solve this. Both of them would try to keep consistency with how it works now because as a user it is quite simple to use and understand. Include the control you want and get rolling.
|
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:
So the proble to solve is how to make these files consolidate code without changing the API.
The text was updated successfully, but these errors were encountered: