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.

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

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

Closed
mattiasmaahl opened this issue Jun 30, 2021 · 6 comments
Closed

Comments

@mattiasmaahl
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Contributor

I can reproduce this. Will have a look.

@matty0ung
Copy link
Contributor

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
Copy link
Collaborator Author

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

@matty0ung
Copy link
Contributor

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
Copy link
Collaborator Author

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

matty0ung added a commit to Brewken/brewken that referenced this issue Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants