search

Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
315 stars 134 forks source link

Crash editing Target Boil Size #817

Closed anthonykelly closed 3 months ago

anthonykelly commented 3 months ago

Full Log: brewtarget.log

I am finding Brewrarget 4.0.3 crashes more often in various situations. This one seems to be fully repeatable for me on Ubuntu and MacOS.

Edit a recipe and change the "Target Boil Size". [09:24:31.290] (1bf3iy2n0g) ERROR : ASSERT: "!this->getTypeInfo().isOptional()" in file ../src/widgets/SmartField.cpp, line 388 [widgets/SmartField.cpp:388]

matty0ung commented 3 months ago

Thanks for the log file and info. I will have a look this evening. One of the changes in 4.0 is that a lot of numerical fields are now optional (ie blank is allowed) where, previously, if there was no available or meaningful value, you had to put 0. The assert is saying that the UI is trying to treat something as optional when it is not. Will likely be a quick fix.

anthonykelly commented 3 months ago

Another example of a crash I'm seeing is editing the Mash Amount [15:19:50.013] (3tdwh1c) ERROR : ASSERT: "!physicalQuantity" in file /Users/akelly/brewtarget/src/tableModels/TableModelBase.h, line 730 [tableModels/TableModelBase.h:730]

matty0ung commented 3 months ago

Thanks for the extra log message.

About to commit small fixes for both of these. Please keep letting me know about other issues you hit.

matty0ung commented 3 months ago

Just to give you a bit more background, another change in 4.0 is that I tried to further reduce the amount of copy-and-paste code between similar bits of the program. Eg the grids at the bottom of the recipe to list fermentables, hops, mash steps, etc are all super similar. Since we were adding new tabs there (Boil and Fermentation), I wanted to pull out as much common code as possible. In the long run, this will mean more consistent behaviour and fewer bugs (honest!).

We are not 100% where I want to be yet -- eg you can see that some fields still blank their contents when you double-click to edit them because the "what should we show during edit" logic is not yet centralised. But it's hopefully moving in the right direction.

anthonykelly commented 3 months ago

Thanks, Matt, I rebuilt from latest and those two issues seem to be resolved.

However, editing the Boil Step Name in the Boil Step Editor produced another crash...

[09:18:29.092] (3tdwh1c) ERROR : ASSERT: "!this->getTypeInfo().isOptional()" in file /Users/akelly/brewtarget/src/widgets/SmartField.cpp, line 390 [widgets/SmartField.cpp:390]

Likewise editing Fermentation Step Name produces... [09:21:21.612] (3tdwh1c) ERROR : ASSERT: "!this->getTypeInfo().isOptional()" in file /Users/akelly/brewtarget/src/widgets/SmartField.cpp, line 390 [widgets/SmartField.cpp:390]

So hopefully the same fix for both.

matty0ung commented 3 months ago

Yes, that's almost certainly the same sort of fix. Will have a look after work.

matty0ung commented 3 months ago

More fixes in https://github.com/Brewtarget/brewtarget/pull/820. (Looks like a bigger change than it really is. I removed some more code duplication, as well as some commented-out code.) I've made some small UI tidy-ups since they were right in front of me.

Please keep bugs coming as you find them!

anthonykelly commented 3 months ago

Thanks, @matty0ung , all of these crashes are fixed now. Tested on macOS and Ubuntu.

matty0ung commented 2 months ago

Fix included in 4.0.4.