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: Fixed uncaught exception when environment/background combination is illegal #28300

Merged
merged 1 commit into from
May 7, 2024

Conversation

ycw
Copy link
Contributor

@ycw ycw commented May 6, 2024

The issues:

  1. Currently it causes error when environment is BACKGROUND and background is not TEXTURE|EQUIRECT; If it happens, users have no clue about the failure because the error is only shown in console: image

  2. Environment will be unlinked from background when recovering from IndexedDB, i.e. environment can never be 'BACKGROUND' after recovering.

This PR fixed 1 by checking If environment/background combination is illegal, then highlight the environment selector in UI

https://github.com/mrdoob/three.js/assets/1063018/4aa0f62a-8166-400d-a767-25859eef1e18

This PR fixed 1 by handling exception of illegal combinations.

This PR fixed 2 by checking if environment.uuid is equal to background.uuid, then set environment value to BACKGROUND

Preview: https://raw.githack.com/ycw/three.js/editor-handle-scene-env-exception/editor/index.html

@ycw ycw force-pushed the editor-handle-scene-env-exception branch from a59f71d to ee0fa8e Compare May 7, 2024 08:52
@Mugen87
Copy link
Collaborator

Mugen87 commented May 7, 2024

This kind of error highlighting is unusual for the editor. How about we fix issue 1 by silently ignoring invalid input for Scene.environment like so:

case 'Background':

	useBackgroundAsEnvironment = true;

	if ( scene.background !== null && scene.background.isTexture ) {

		scene.environment = scene.background;
		scene.environment.mapping = THREE.EquirectangularReflectionMapping;
		scene.environmentRotation.y = scene.backgroundRotation.y;

	}

	break;

@ycw ycw force-pushed the editor-handle-scene-env-exception branch from ee0fa8e to ca8a256 Compare May 7, 2024 12:02
@ycw
Copy link
Contributor Author

ycw commented May 7, 2024

ok, removed the highlight part

@ycw ycw force-pushed the editor-handle-scene-env-exception branch from ca8a256 to c4aeaea Compare May 7, 2024 12:33
@ycw ycw force-pushed the editor-handle-scene-env-exception branch from c4aeaea to d6923b3 Compare May 7, 2024 12:37
@Mugen87 Mugen87 added this to the r165 milestone May 7, 2024
@Mugen87 Mugen87 merged commit 6d53d23 into mrdoob:dev May 7, 2024
8 of 12 checks passed
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