search

Brewtarget / brewtarget

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

Inventory error?! Unknown signal. Trying to add inventory to yeasts. #601

Closed mattiasmaahl closed 3 years ago

mattiasmaahl commented 3 years ago

I'm trying to add inventory to the Yeasts, one of the included ones (Danstar - Nottingham). But Brewtarget crashes abruptly with the following error: bild

bild

can anyone shed some light on this as I don't fully follow the redo/undo methods?

mattiasmaahl commented 3 years ago

So far I'm having problems with Yeast, Hops and Misc. all with the above error. Fermentables was OK to add to inventory for some reason.

This could be because of my database as well, have to do more testing tomorrow.

matty0ung commented 3 years ago

I can reproduce this. Will have a look.

matty0ung commented 3 years ago

OK, I have a fix for this. Will backport it to Brewtarget and submit in the next day or so. (Need a couple of tweaks for the backport as NamedEntityWithInventory doesn't yet exist in BT :o>)

mattiasmaahl commented 3 years ago

Nice, I'll continue my work with inventory printout using fermentables for now then awaiting your fix. 😊

matty0ung commented 3 years ago

The crash was caused by an invalid property name in YeastTabelModel.cpp (and equivalent places for other things with inventory). I've changed the hard-coded string property names for symbolic ones (so we should get compiler errors in the future for bad property names), as has already been done in most other parts of the code. I also pulled out the common inventory functions from Yeast, Hop, etc to an intermediate parent class (NamedEntityWithInventory). So the crashing seems to stop now...

However, there is still an issue that we're not always setting the inventory ID, which means inventory amounts aren't getting stored. There may be some small fix in Brewtarget that addresses this, and I'm just not instantly seeing it. Otherwise, when I bring the new DB layer back, there's new code (in a new Inventory class) that does all the right things.

mattiasmaahl commented 3 years ago

So the error has been solved. I'll close this issue. Solved in #602