[PATCHES] 4.5.5

Dirk Hohndel dirk at hohndel.org
Wed Mar 23 07:52:43 PDT 2016


On Wed, Mar 23, 2016 at 08:47:36AM +0100, Robert Helling wrote:
> 
> you caught me here but your „solution“ actually makes it worse:

Ha.

> The problem is when switching from PSI to bar: You start, say with a value of 580psi (roughly 40bar). Then you change units, this makes the above code execute. Now you reduce the maximum to 100 which changes the value displayed from 580 to 100. This in turn sends a value changed signal. The receiving slots checks units and finds „bar“, and so interprets the 100 as 100bar  and sets the prefs.reserve_gas to 100000. And only then the above code is further executed but now thinks 100bar is what the user wants.

Oops. I tested this only in the other direction. And should of course have
tested both directions.

> When I wrote this yesterday, I had (somewhat) reasonable values for the maximum first (after I encountered that there is a maximum, probably coming from the .ui file which is way to low for a psi value). The I ran into the issue I described above. But as you noted, this was around midnight and I was not able to solve this properly. So I just thought „what the heck“ and made the maximum values so big that they did not interfere anymore with reasonably sized reserves. This obviously was a hack of somebody who desperately wanted to go to bed and you caught me.

So I did half of my maintainer job :-)

> What I should have done instead would have been either
> 
> a) Move the setMaximim() call after the setValue() call in the metric branch which avoids the above issue (my thinking was not clear enough to realize this would have been the appropriate thing)

That sounds like a good idea.
> 
> or
> 
> b) Get rid of the maximum value properly (not by making it ridiculously big but by also deleting it from the .ui file). In any case, what is it good for? Why do we want to prevent the user from doing stupid things? Because we could get crazy values when turning the mouse wheel? And what is an appropriate maximum? 100bar since no sane person would keep a reserve that big? Or 300bar since this is roughly the highest pressure we would ever encounter no matter if it makes sense as a reserve?

I think having a maximum there is reasonable. We could make it
300bar/5000psi and be done with it. Who knows, maybe there is GUI++ that
says "never use more than half your tank because an Asteroid could hit the
earth while you are diving".

> Please let me know your preference and I will send a new patch. Do you want one with —amend or a second one on top of your modification?

Since this was already pushed out, it has to be a second patch that fixes
things. Otherwise this would change a git repository that others have
pulled from and pulls wouldn't be 'fast forward' anymore.

/D



More information about the subsurface mailing list