This PR was nearly ready to go, but needed a couple of tests :) That stalled out, so it was temporarily adopted, with some new tests in place. The original author noticed that, said hi, and now the tests are back in the original PR.
Great!
This is a nice bit of usability work for our specialized Entity containers,
allowing them to be used with newtyped Entitys, like you might use to add type-safety.
I like the notion, and the quality is good. Merging!
Wait, I tried to merge this yesterday and CI stalled out? Ugh, Github please. Updating the branch and letting automerge do its thing.
Oh my god dealing with fixed timesteps is so, so absurdly complicated. What a nasty little ball of edge cases. Bevy tries to make it seamless for users, but that means we are constantly fixing little bugs in our code instead.
In this case, we weren't clamping appropriately after accounting for scale. Regression tests? Wow, Francois, that's real engineering <3
I agree with kfc35's comment on the secondary bug, but also agree with mockersf that this is not this PR's concern. Spinning out a little good-first-issue: bevyengine/bevy#24715 Bam, easy.
Anyways, fix looks great, and there's tests. Merging.
Wow, wgpu, you're actually going to enforce that we ship valid shader code?! That's against the whole ethos of shader languages!
DX12 may be fairly second-class at this point, but second-class passengers have a ticket to ride too!
Sure, this seems like a very straightforward fix,
and beicause is a skilled rendering engineer whose judgement I trust here.
Nice to see the direct collaboration with wgpu to make this even better.
Merging.
+57/-248. Looks like the deduplication was successful.
Not crazy over-engineered, and the logic looks like it's faithfully translated. I'm happy with this, merging.
This type of PR really demonstrates the value of open source + open code review. How much maintainer time did we spend on this? Literally 1 minute?
And now our tests are 1% nicer, and 3 other people have learned a little bit about Bevy and Rust! Repeat over and over again :)
Examples: not just for learning. Bevy engine devs use our examples all the time to test changes and check for problems.
To make that easier, the examples need to be configurable. We use CLI arguments in some places, key-codes in others, and little on-screen widgets in others still.
I would like to do a consistency pass on this to make these all use little on-screen toggles etc now that we have them, but this PR is plenty-fine for now.
Merging: I hope this makes developing Solari easier.
Oh rendering, home of the acronyms. Solari is Bevy's raytraced global illumination lighting engine. Like all modern raytraced lighting, Solari is fundamentally stochastic, sampling from an unfeasibly large space and trying to fill in the gaps using sophisticated statistical methods.
MIS is Multiple Importance Sampling, a technique used to combine sampling strategies in order to improve estimations.
PDF is Probability Density Function, the "shape" of the probability space you're trying to estimate.
NEE is Next Event Estimation, a direct-lighting technique that explicitly samples a light source.
NdotV is Normal dot product View. The cosine between a surface normal and a viewing/sampling direction, incidentally used for the math here.
The actual bug is that we were missing a conversion factor, resulting in the PDFs from the two sampling strategies being combined with different units. Unfortunately, uom is not shader-language compatible.
Great, now that I've spent 15 minutes looking up what's going on here, I understand the problem and fix! Merging.
A simple enough UI bug fix, for a latent bug manifested in our new scroll-box code.
This sort of thing is everywhere when you're building out UI, and the best way to find and fix these problems is still to endlessly play with weirder and weirder widgets and layouts, trying to look for unusual problems.
Over the past year or so sentiment on bevy_ui has gone from "what a sad joke"
to "surprisingly usable actually??", and while the widgets and obsevers and bsn steal the show,
much of the difference actually comes from the hundreds of tiny bug fixes,
mostly from ickshonpe <3
Very happy to squash another. Merging.
Deprecation warnings are a great tool for Rust, and make it super easy for folks to update their Bevy code between versions.
We always recommend going up one version at a time (and our migration guides are structured that way). As a result, we can get rid of the deprecated code immediately after shipping a new Bevy version, unless there are serious problems with our migration path.
+5/-526 feels great. Yeet!
Right, these new color types should be in the prelude like all the others. Merging.
The other cleanup is nice too; sure we'll toss that in.
Speaking of new color types: fix some NaN-generating bugs during edge cases!
Ooh, and we've even added white_plus_epsilon and black_plus_epsilon to our test code. Lovely stuff.
A couple lines of fix, a hundred lines of tests. As it should be. That makes me way more confident to merge these nasty math-heavy fixes. Merging.
A nice trivial builder method for lights. Sure, I'm down, the snippet in the PR description seems nice.
Merging.
More people doing serious feathers work! Yay! The author is working on a light mode for feathers, and the fact that our radio buttons can't have a background color set was a real wart.
Sure, that should be themeable. Merging.
Real 👀 at shipping a light-mode theme out-of-the-box for feathers soon though.
Pick up a bug fix in one of our dependencies, released in a patch version. This is a real issue, reported to Bevy, even though it wasn't our bug.
cargo update can also fix this. Is the stricter version requirement necessary?
True! These are semver-compatible versions.
[S]trictly speaking, it is not required to update the version number. Still, the patches resolve platform-specific compile errors (critical) and if it doesn't matter that much as it automatically gets resolved by cargo, then why bother with the patch numbers in the first place?
Also true. I think it's a bit silly that we specify minor version numbers like this when they just largely get ignored by cargo anyways, but eh, this is a bit better if we're going to be specifying that at all.
Fine, merging.
Downgrade a noisy, non-actionable warning to a lower level! Sure, I like that sort of idea.
You can also filter these warnings locally, but this is good to clean up for everyone. You very much should not be forking things over noisy logs :p
Merging as trivial.
The author noticed that our screenshot for the diagnostics overlay looked off! They took a look at the code, and it's a one line fix. D'oh.
Merging.
Oof, we're missing an import and so some of our shader code just silently failed?? I hate shader languages. What the hell guys, real programming languages figured this out decades ago.
Merging and backporting...
A simple-yet-complex rendering bug, with incorrect rendering of certain assets on web only. 3 line fix, and I'm sure a hell of a lot of effort identifying the actual cause.
This sort of investigation is why I feel so strongly that we need to make sure that various developer tools (like the Bevy Editor) are available to engine devs too. Which means that it's mono-repo time!
Very nice video of before/after: what an excellent demonstration. Merging and backporting.