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

Metadata with long number causes hard crash #140

Open
bbulkow opened this issue Nov 19, 2022 · 5 comments
Open

Metadata with long number causes hard crash #140

bbulkow opened this issue Nov 19, 2022 · 5 comments
Assignees
Labels
2 MediumPriority - chromatik nice to fix but can go live without it

Comments

@bbulkow
Copy link
Collaborator

bbulkow commented Nov 19, 2022

With a metadata like this:
"meta": {
"name": "bench-1",
"base_x": 40,
"base_y": 0,
"base_z": 696,
"ry": 230
}
we had written some math that caused the value 229.999999999997 (or similar), and that caused crashes when playing different files. It didn't crash immediately, but it did crash hard (required force quit).

@bbulkow bbulkow added the 1 HighPriority - chromatik really should get fixed label Nov 19, 2022
@bbulkow bbulkow added 2 MediumPriority - chromatik nice to fix but can go live without it and removed 1 HighPriority - chromatik really should get fixed labels Nov 19, 2022
@bbulkow
Copy link
Collaborator Author

bbulkow commented Nov 19, 2022

We have worked around the problem by not loading metadata like this.

@mcslee
Copy link
Collaborator

mcslee commented Nov 19, 2022 via email

@mcslee
Copy link
Collaborator

mcslee commented Nov 19, 2022

This very likely will have been these lines:
https://github.com/squaredproject/Entwined/blob/master/chromatik/src/main/java/entwined/core/CubeManager.java#L61
https://github.com/squaredproject/Entwined/blob/master/chromatik/src/main/java/entwined/pattern/misko/SyncSpinner.java#L42

Number parsing methods like Integer.parseInt() can throw unchecked NumberFormatException and in this case I'd bet that's what happened here.

Would be safer to have those method use Double.parseDouble() and cast to an integer.

Screen Shot 2022-11-19 at 9 01 59 AM

@mcslee
Copy link
Collaborator

mcslee commented Nov 19, 2022

FYI also a best practice to surround with an explicit try/catch when using number parsing methods, just in case you get garbage strings in there somehow. I find it annoying that they made these unchecked exceptions in Java. It's a pretty easy way to get an unexpected RuntimeException

@cswales
Copy link
Collaborator

cswales commented Nov 19, 2022

@mcslee - Yep. Fast coding and laziness on my fault. Apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 MediumPriority - chromatik nice to fix but can go live without it
Projects
None yet
Development

No branches or pull requests

3 participants