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

Add common file with GLMAT_ARRAY_TYPE, GLMAT_EPSILON and GLMAT_RANDOM #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

manuelcabral
Copy link

No description provided.

@mattdesl
Copy link
Member

mattdesl commented May 9, 2015

Thanks for the PR!

One of the difficulties with this is that it exposes these globals to the end-user, which could lead to some problems across many modules. For example, module A might rely on GLMAT_ARRAY_TYPE to be an Array (e.g. using [].map et al), whereas module B might rely on it being a typed array.

Not sure what the best approach is for this, maybe @hughsk and @mikolalysenko have some ideas.

@manuelcabral
Copy link
Author

Thank you for the reply and the very useful module!

I'm not sure I understand the difficulty. Isn't this also an issue when using the original gl-matrix?

@mattdesl
Copy link
Member

mattdesl commented May 9, 2015

The problem becomes more pronounced in modular programming, where you have dozens of dependents on a module, and then dozens more transitive and unlisted dependents. These modules all need to work together; e.g.

  • module A and B both rely on gl-mat4
  • module C relies on A and B
  • module D relies on module C

If either A or B changed the internal state of gl-mat4 it could have catastrophic effects across dozens of dependents.

This was never really a problem with the original vision of gl-matrix since most consumers of it are not working with a dependency depth greater than 1.

@manuelcabral
Copy link
Author

I understand now. What if common.js exported functions which just return the values, not allowing them to be changed?

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.

2 participants