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

VRMLLoader: Allow minus in Identifier #20321

Merged
merged 1 commit into from
Sep 11, 2020
Merged

VRMLLoader: Allow minus in Identifier #20321

merged 1 commit into from
Sep 11, 2020

Conversation

tuarrep
Copy link
Contributor

@tuarrep tuarrep commented Sep 11, 2020

Reopening of #18298

@mrdoob mrdoob added this to the r121 milestone Sep 11, 2020
@mrdoob mrdoob merged commit 05d5f6d into mrdoob:dev Sep 11, 2020
@mrdoob
Copy link
Owner

mrdoob commented Sep 11, 2020

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Sep 11, 2020

@tuarrep Did that fix it? I tried loading one of your wrl files and I'm getting this:

Screen Shot 2020-09-11 at 10 50 14 AM

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 11, 2020

The jsm module probably needs an update first.

@mrdoob
Copy link
Owner

mrdoob commented Sep 11, 2020

I updated it. And now webgl_loader_vrml is failing.

@tuarrep are you able to investigate this?

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 11, 2020

@tuarrep Contributions should always be tested with existing examples. It's not acceptable to introduce a breakage. If you can't manage to fix this issue, the PR has to be reverted.

@tuarrep
Copy link
Contributor Author

tuarrep commented Sep 11, 2020

@Mugen87

It's not my responsibility to merge. If it breaks, revert it, no problem.

@mrdoob I can investigate, I need some time to do so. As the patch was written in January, I need to recall all the details.
I've a working patched code on a project, I'll test on it.

@mrdoob
Copy link
Owner

mrdoob commented Sep 11, 2020

It's all good! I'll revert in the meantime.

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.

3 participants