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.

Sign up for GitHub

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

Jump to bottom

Remove ambient light as a resource #17367

Open
alice-i-cecile opened this issue Jan 14, 2025 · 5 comments
Open

Remove ambient light as a resource #17367

alice-i-cecile opened this issue Jan 14, 2025 · 5 comments
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through

Comments

@alice-i-cecile
Copy link
Member

I'd be in favor of marking the resource as deprecated, but we can do that later.

Originally posted by @pcwalton in #17343 (review)

I agree with this: there should be one clear way to do this.
@alice-i-cecile
Copy link
Member Author

I don't think we can mark the Resource implementation as deprecated due to limitations in Rust. See rust-lang/rust#39935.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through labels Jan 14, 2025
@IceSentry
Copy link
Contributor

IceSentry commented Jan 14, 2025 •
edited
Loading

I'm not sure about that. It's used as a way to configure a global default now. Which I think makes sense to keep?

Like, if someone wants no ambient light and we remove the resource, they would need to manually set it to 0 on all cameras if we remove the resource.

@alice-i-cecile
Copy link
Member Author

Can't we just treat "no value set on camera" as 0?

@mockersf
Copy link
Member

no strong opinion for me, but if we remove it as a resource we should probably reverse the requirement and instead of ambient light requiring a camera, having camera require an ambient light

@IceSentry
Copy link
Contributor

IceSentry commented Jan 14, 2025 •
edited
Loading

Can't we just treat "no value set on camera" as 0?

It isn't just about setting it to 0 though. If for example someone wanted a non-default ambient light applied to every camera and disable it only on one then they would again need to update all their camera.

To be clear, it would probably be fine for most people, but that would be one reason to keep it around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@alice-i-cecile @IceSentry @mockersf