search

Brewtarget / brewtarget

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

Boil Time not saved when you edit a copied recipe #688

Closed jeroen79 closed 1 year ago

jeroen79 commented 1 year ago

I am using V3.0.3 and if i change the boil time and switch recipes the change is lost.

matty0ung commented 1 year ago

Sorry to hear you're having problems. I'm not able to reproduce this on my local install. Could you upload the log files in case that might give us a clue? (If you're running on Linux, it's usually ~/.config/brewtarget/brewtarget.log by default.) If you were also able to turn on debug logging from the GUI (Tools > Options > Logging > Logging Level = Detailed) before reproducing, that would be great.

jeroen79 commented 1 year ago

I turened on the log but wasn't able to reproduce it at first, but after some testing and recalling steps, it seems to happen only when you copy a recepie and edit the copy.

matty0ung commented 1 year ago

OK, thanks, I can see it now. I will take a look, though it might be next weekend before I really have any time.

I think your workaround in the meantime is to close and re-run the program after creating the copy. Then the changes to the copy will stick. At least, that's how it seems to behave on my install.

matty0ung commented 1 year ago

From what I can tell, the issue is to do with slots and signals. When you update the boil time in the user interface, it sends a signal to the MainWindow class, which then either updates the recipe (if it doesn't have any equipment) or the recipe's equipment (if it does). In the latter case the equipment then sends an "I have changed" signal to the recipe so the recipe knows to update its boil time. For some reason this last signal is not getting received. I'm guessing that when we create a copy of the recipe we're doing something in the wrong order.

TLDR: I think I've narrowed down where to look in the code. Will update when I have more info.

matty0ung commented 1 year ago

OK, pretty sure I found the problem. We're not doing all the same signal connecting in Recipe::Recipe(Recipe const & other) that we are in Recipe::connectSignals(). The former is used when we make a copy of a Recipe; the latter connects all the signals for all the Recipes at start-up. I'm going to refactor the code so that we have exactly one place that connects signals for a recipe and then have these two functions call it. Patch coming.

matty0ung commented 1 year ago

Closing now that https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.5 is available.