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

Editor: Runtime error in history. #28395

Closed
Mugen87 opened this issue May 16, 2024 · 6 comments · Fixed by #28405
Closed

Editor: Runtime error in history. #28395

Mugen87 opened this issue May 16, 2024 · 6 comments · Fixed by #28405
Milestone

Comments

@Mugen87
Copy link
Collaborator

Mugen87 commented May 16, 2024

Description

When using persistent history, a runtime error is thrown when deleting an object and refreshing the page.

RemoveObjectCommand.js:17 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'name')
at new RemoveObjectCommand (RemoveObjectCommand.js:17:79)
at History.fromJSON (History.js:204:16)
at Editor.fromJSON (Editor.js:674:16)
at async index.html:115:7

Reproduction steps

  1. Enable persistent history in the settings tab.
  2. Add a box.
  3. Delete it.
  4. Refresh the site and check browser console.

Code

Live example

https://rawcdn.githack.com/mrdoob/three.js/dev/editor/index.html

Screenshots

No response

Version

r165dev

Device

Desktop

Browser

Chrome

OS

MacOS

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 16, 2024

/ping @ycw

@Mugen87 Mugen87 added this to the r165 milestone May 16, 2024
@ycw
Copy link
Contributor

ycw commented May 16, 2024

Already solved by "Editor: Fixed recovery system" #28345

You can try it at https://raw.githack.com/ycw/three.js/editor-commands-defer-init/editor/index.html

Was asked to split #28345 into small PRs for reviews, I did that but there's no signals after that :D

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 16, 2024

Which of the new PRs fixes this particular issue?

Was asked to split #28345 into small PRs for reviews, I did that but there's no signals after that :D

Please keep in mind that our review time is limited and the editor related PRs of you and @linbingquan claimed a considerable amount of reviewing and testing bandwidth in the last week. We also want to review other PRs and work on our own stuff so please do not expect too much attention.

@Mugen87 Mugen87 added Bug and removed Regression labels May 16, 2024
@ycw
Copy link
Contributor

ycw commented May 16, 2024

#28360
#28361
#28362

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 16, 2024

After testing it seems #28360 is sufficient to fix this particular issue.

@ycw
Copy link
Contributor

ycw commented May 16, 2024

Since the issue isn't material-related #28362 nor about SetGeometryCommand #28361, so yes, its related to #28360 only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants