Error message for setting shaderWarmUp too late #30145
Conversation
Developers may get confused by setting PaintingBinding.shaderWarmUp in the wrong place. The added assert and error message help avoid that.
|
@InMatrix : please let me know if the error message conforms your UX research results :) |
| assert(_instance == null, | ||
| 'PaintingBinding.shaderWarmUp should only be set before the ' | ||
| 'PaintingBinding singleton instance is initialized.\n\n' | ||
| 'Set it after init would not affect how shaders are warmed up.\n\n' |
There was a problem hiding this comment.
small nit:
| 'Set it after init would not affect how shaders are warmed up.\n\n' | |
| 'Setting it after init does not affect how shaders are warmed up.\n\n' |
"Set" here makes it sound like a command so it's confusing as is, because I think this sentence is really talking about the times when this is set after init and not telling the reader to set it after init. "Setting" changes it to a gerund and the subject of this sentence, which makes more sense. Then the "would" to "does" here change follows to match that.
| /// * [ShaderWarmUp], the interface of how this warm up works. | ||
| static ShaderWarmUp shaderWarmUp = const DefaultShaderWarmUp(); | ||
| static ShaderWarmUp get shaderWarmUp => _shaderWarmUp; | ||
| static void set shaderWarmUp(ShaderWarmUp value) { |
There was a problem hiding this comment.
nit: This is failing the analyzer. "Avoid return types on setters"
There was a problem hiding this comment.
Thank you very much for the suggestion! Done.
|
@liyuqian Thanks for pinging me. Being unfamiliar with the code, I can't really tell if the message is sufficient. But it's certainly a step towards the right direction. |
This reverts commit ffbb335. Reason for revert: flutter driver tests may have bugs in getting first frame. Revert this until we figure out what happened.
4 participants
Description
Developers may get confused by setting
PaintingBinding.shaderWarmUpinthe wrong place. The added assert and error message help avoid that.
Related Issues
#813
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?