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

Remove GI methods in parentheses from light baking options #85219

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Nov 22, 2023

Presently, on lights, the "Bake Mode" property has 3 options:

  • Disabled
  • Static (VoxelGI/SDFGI/LightmapGI)
  • Dynamic (VoxelGI/SDFGI)

I think this is potentially confusing (it confused me :-)) because it makes it sound like lights marked "Dynamic (VoxelGI/SDFGI)" won't be baked into LightmapGI lightmaps.

However, they are baked into LightmapGI lightmaps, but only the bounced light, not the direct light.

So, in this case, I think it'd be best to just remove the GI methods from the option names, since all the options do apply to all GI methods.

There are a number of other properties with similar options, for example, gi_mode on VisualInstance3D, but in that case I think it's OK because it's true (if marked as "Dynamic (VoxelGI only)" that visual instance won't receive light from LightmapGI). The same goes for the meshes/light_baking option from ResourceImporterScene.

UPDATE: Per @Calinou's suggestion, this PR now removes the GI method names from all light baking options.

@Calinou
Copy link
Member

Calinou commented Nov 22, 2023

(if marked as "Dynamic (VoxelGI only)" that visual instance won't receive light from LightmapGI).

The dynamic object can still receive indirect light from LightmapGI's dynamic probe system (direct light is always real-time, even for lights marked as Static). As a result, I'd go for removing that hint for GI mode as well.

In general, there are many nuances that short hints can't cover, so it's better to refer people to the documentation to know what will happen with each GI mode exactly.

@Calinou Calinou added this to the 4.3 milestone Nov 22, 2023
@dsnopek dsnopek requested a review from a team as a code owner November 22, 2023 14:43
@dsnopek
Copy link
Contributor Author

dsnopek commented Nov 22, 2023

Alright, I've updated the PR to remove the GI method names from all the light baking options

@dsnopek dsnopek changed the title Remove GI methods in parentheses from the light_bake_mode property Remove GI methods in parentheses from light baking options Nov 22, 2023
@dsnopek dsnopek added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
@YuriSizov YuriSizov merged commit 13dd72c into godotengine:master Dec 19, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 10, 2024
@dsnopek dsnopek deleted the light-bake-mode-no-gi-method branch July 22, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants